diff --git a/core/src/main/java/org/springframework/security/context/HttpSessionSecurityContextRepository.java b/core/src/main/java/org/springframework/security/context/HttpSessionSecurityContextRepository.java
index 9ddc7f8c2a..e2280a4880 100644
--- a/core/src/main/java/org/springframework/security/context/HttpSessionSecurityContextRepository.java
+++ b/core/src/main/java/org/springframework/security/context/HttpSessionSecurityContextRepository.java
@@ -21,7 +21,7 @@ import org.springframework.util.ReflectionUtils;
* method (using the key {@link #SPRING_SECURITY_CONTEXT_KEY}). If a valid SecurityContext
cannot be
* obtained from the HttpSession
for whatever reason, a fresh SecurityContext
will be created
* and returned instead. The created object will be an instance of the class set using the
- * {@link #setContextClass(Class)} method. If this hasn't been set, a {@link SecurityContextImpl} will be returned.
+ * {@link #setSecurityContextClass(Class)} method. If this hasn't been set, a {@link SecurityContextImpl} will be returned.
*
* When saveContext is called, the context will be stored under the same key, provided *
+ * If the session is null, the context object is null or the context object stored in the session + * is not an instance of SecurityContext, a new context object will be generated and + * returned. + *
+ * If cloneFromHttpSession is set to true, it will attempt to clone the context object first + * and return the cloned instance. + */ public SecurityContext loadContext(HttpRequestResponseHolder requestResponseHolder) { HttpServletRequest request = requestResponseHolder.getRequest(); HttpServletResponse response = requestResponseHolder.getResponse(); @@ -96,16 +106,7 @@ public class HttpSessionSecurityContextRepository implements SecurityContextRepo } } - - /** - * Gets the security context from the session (if available) and returns it. - *
- * If the session is null, the context object is null or the context object stored in the session - * is not an instance of SecurityContext it will return null. - *
- * If cloneFromHttpSession is set to true, it will attempt to clone the context object - * and return the cloned instance. * * @param httpSession the session obtained from the request. */ @@ -216,6 +217,12 @@ public class HttpSessionSecurityContextRepository implements SecurityContextRepo this.cloneFromHttpSession = cloneFromHttpSession; } + /** + * If set to true (the default), a new session will be created if to store the security context if it is determined + * that it's contents are different from the default. + * + * @param allowSessionCreation + */ public void setAllowSessionCreation(boolean allowSessionCreation) { this.allowSessionCreation = allowSessionCreation; } @@ -272,46 +279,14 @@ public class HttpSessionSecurityContextRepository implements SecurityContextRepo HttpSession httpSession = request.getSession(false); if (httpSession == null) { - if (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"); - } - } else { - // Generate a HttpSession only if we need to - - if (!allowSessionCreation) { - if (logger.isDebugEnabled()) { - logger.debug("The HttpSession is currently null, and the " - + "HttpSessionContextIntegrationFilter is prohibited from creating an HttpSession " - + "(because the allowSessionCreation property is false) - SecurityContext thus not " - + "stored for next request"); - } - } else if (!contextObject.equals(context)) { - if (logger.isDebugEnabled()) { - logger.debug("HttpSession being created as SecurityContextHolder contents are non-default"); - } - - try { - httpSession = request.getSession(true); - } catch (IllegalStateException e) { - // Response must already be committed, therefore can't create a new session - } - - } else { - if (logger.isDebugEnabled()) { - logger.debug("HttpSession is null, but SecurityContextHolder has not changed from default: ' " - + context - + "'; not creating HttpSession or storing SecurityContextHolder contents"); - } - } - } + httpSession = createNewSessionIfAllowed(context); } // If HttpSession exists, store current SecurityContextHolder contents but only if // the SecurityContext has actually changed (see JIRA SEC-37) if (httpSession != null && context.hashCode() != contextHashBeforeChainExecution) { - // See SEC-766 + // See SEC-776 + // TODO: Move this so that a session isn't created if user is anonymous if (authenticationTrustResolver.isAnonymous(context.getAuthentication())) { if (logger.isDebugEnabled()) { logger.debug("SecurityContext contents are anonymous - context will not be stored in HttpSession. "); @@ -325,5 +300,52 @@ public class HttpSessionSecurityContextRepository implements SecurityContextRepo } } } + + private HttpSession createNewSessionIfAllowed(SecurityContext context) { + if (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"); + } + + return null; + } + + if (!allowSessionCreation) { + if (logger.isDebugEnabled()) { + logger.debug("The HttpSession is currently null, and the " + + "HttpSessionContextIntegrationFilter is prohibited from creating an HttpSession " + + "(because the allowSessionCreation property is false) - SecurityContext thus not " + + "stored for next request"); + } + + return null; + } + // Generate a HttpSession only if we need to + + if (contextObject.equals(context)) { + if (logger.isDebugEnabled()) { + logger.debug("HttpSession is null, but SecurityContext has not changed from default: ' " + + context + + "'; not creating HttpSession or storing SecurityContext"); + } + + return null; + } + + if (logger.isDebugEnabled()) { + logger.debug("HttpSession being created as SecurityContext is non-default"); + } + + try { + return request.getSession(true); + } catch (IllegalStateException e) { + // Response must already be committed, therefore can't create a new session + logger.warn("Failed to create a session, as response has been committed. Unable to store" + + " SecurityContext."); + } + + return null; + } } }