From 62c832e366f7ab5fd922c9fa7ffc93555980b92d Mon Sep 17 00:00:00 2001 From: Vishal Puri Date: Fri, 27 Apr 2007 07:35:11 +0000 Subject: [PATCH] SEC-423: Fixed IllegalArguemntException being thrown by checking for null contextFromSessionObject --- .../HttpSessionContextIntegrationFilter.java | 654 ++++++++++-------- 1 file changed, 354 insertions(+), 300 deletions(-) diff --git a/core/src/main/java/org/acegisecurity/context/HttpSessionContextIntegrationFilter.java b/core/src/main/java/org/acegisecurity/context/HttpSessionContextIntegrationFilter.java index b946495538..60ab6babca 100644 --- a/core/src/main/java/org/acegisecurity/context/HttpSessionContextIntegrationFilter.java +++ b/core/src/main/java/org/acegisecurity/context/HttpSessionContextIntegrationFilter.java @@ -33,323 +33,377 @@ import org.springframework.beans.factory.InitializingBean; import org.springframework.util.Assert; import org.springframework.util.ReflectionUtils; - /** - * Populates the {@link SecurityContextHolder} with information obtained from the HttpSession. - * + * Populates the {@link SecurityContextHolder} with information obtained from + * the HttpSession. + * *

- * The HttpSession will be queried to retrieve the SecurityContext that should be - * stored against the SecurityContextHolder for the duration of the web request. At the end of the web - * request, any updates made to the SecurityContextHolder will be persisted back to the + * The HttpSession will be queried to retrieve the + * SecurityContext that should be stored against the + * SecurityContextHolder for the duration of the web request. At + * the end of the web request, any updates made to the + * SecurityContextHolder will be persisted back to the * HttpSession by this filter. *

*

- * If a valid SecurityContext cannot be obtained from the HttpSession for whatever - * reason, a fresh 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 + * If a valid SecurityContext cannot be obtained from the + * HttpSession for whatever reason, a fresh + * 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}. *

*

- * No HttpSession will be created by this filter if one does not already exist. If at the end of - * the web request the HttpSession does not exist, a HttpSession will only be created - * if the current contents of the SecurityContextHolder are not {@link - * 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. 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). + * No HttpSession will be created by this filter if one does not + * already exist. If at the end of the web request the HttpSession + * does not exist, a HttpSession will only be created if + * the current contents of the SecurityContextHolder are not + * {@link 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. 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). *

*

- * This filter will only execute once per request, to resolve servlet container (specifically Weblogic) - * incompatibilities.

+ * This filter will only execute once per request, to resolve servlet container + * (specifically Weblogic) incompatibilities. + *

*

- * 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 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. Please note that if {@link - * #forceEagerSessionCreation} is true, the allowSessionCreation must also be - * true (setting it to false will cause a startup time error). + * 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 + * 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. 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 + * 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 * @author Patrick Burleson - * @version $Id$ + * @version $Id: HttpSessionContextIntegrationFilter.java 1784 2007-02-24 + * 21:00:24Z luke_t $ */ public class HttpSessionContextIntegrationFilter implements InitializingBean, Filter { - //~ 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 ================================================================================================ - - 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. 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; - - /** - * Indicates whether the SecurityContext will be cloned from the HttpSession. The - * default is to simply reference (ie the default is false). The default may cause issues if - * concurrent threads need to have a different security identity from other threads being concurrently processed - * that share the same HttpSession. In most normal environments this does not represent an issue, - * as changes to the security identity in one thread is allowed to affect the security identitiy in other - * threads associated with the same HttpSession. For unusual cases where this is not permitted, - * change this value to true and ensure the {@link #context} is set to a SecurityContext - * that implements {@link Cloneable} and overrides the clone() method. - */ - private boolean cloneFromHttpSession = false; - - public boolean isCloneFromHttpSession() { - return cloneFromHttpSession; - } - - public void setCloneFromHttpSession(boolean cloneFromHttpSession) { - this.cloneFromHttpSession = cloneFromHttpSession; - } - - public HttpSessionContextIntegrationFilter() throws ServletException { - this.contextObject = generateNewContext(); - } - - //~ Methods ======================================================================================================== - - public void afterPropertiesSet() throws Exception { - if ((this.context == null) || (!SecurityContext.class.isAssignableFrom(this.context))) { - throw new IllegalArgumentException( - "context must be defined and implement SecurityContext " - + "(typically use org.acegisecurity.context.SecurityContextImpl; existing class is " - + this.context + ")"); - } - - if ((forceEagerSessionCreation == true) && (allowSessionCreation == false)) { - throw new IllegalArgumentException( - "If using forceEagerSessionCreation, you must set allowSessionCreation to also be true"); - } - } - - /** - * Does nothing. We use IoC container lifecycle services instead. - */ - public void destroy() {} - - public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) - throws IOException, ServletException { - if ((request != null) && (request.getAttribute(FILTER_APPLIED) != null)) { - // ensure that filter is only applied once per request - chain.doFilter(request, response); - } else { - if (request != null) { - request.setAttribute(FILTER_APPLIED, Boolean.TRUE); - } - - HttpSession httpSession = null; - boolean httpSessionExistedAtStartOfRequest = false; - - try { - httpSession = ((HttpServletRequest) request).getSession(forceEagerSessionCreation); - } catch (IllegalStateException ignored) {} - - if (httpSession != null) { - httpSessionExistedAtStartOfRequest = true; - - Object contextFromSessionObject = httpSession.getAttribute(ACEGI_SECURITY_CONTEXT_KEY); - - // Clone if required (see SEC-356) - if (cloneFromHttpSession) { - Assert.isInstanceOf(Cloneable.class, contextFromSessionObject, - "Context must implement Clonable and provide a Object.clone() method"); - try { - Method m = contextFromSessionObject.getClass().getMethod("clone", new Class[] {}); - if (!m.isAccessible()) { - m.setAccessible(true); - } - contextFromSessionObject = m.invoke(contextFromSessionObject, new Object[] {}); - } catch (Exception ex) { - ReflectionUtils.handleReflectionException(ex); - } - } - - if (contextFromSessionObject != null) { - if (contextFromSessionObject instanceof SecurityContext) { - if (logger.isDebugEnabled()) { - logger.debug("Obtained from ACEGI_SECURITY_CONTEXT a valid SecurityContext and " - + "set to SecurityContextHolder: '" + contextFromSessionObject + "'"); - } - - SecurityContextHolder.setContext((SecurityContext) contextFromSessionObject); - } else { - if (logger.isWarnEnabled()) { - logger.warn("ACEGI_SECURITY_CONTEXT did not contain a SecurityContext but contained: '" - + contextFromSessionObject - + "'; are you improperly modifying the HttpSession directly " - + "(you should always use SecurityContextHolder) or using the HttpSession attribute " - + "reserved for this class? - new SecurityContext instance associated with " - + "SecurityContextHolder"); - } - - SecurityContextHolder.setContext(generateNewContext()); - } - } else { - if (logger.isDebugEnabled()) { - logger.debug("HttpSession returned null object for ACEGI_SECURITY_CONTEXT - new " - + "SecurityContext instance associated with SecurityContextHolder"); - } - - SecurityContextHolder.setContext(generateNewContext()); - } - } else { - if (logger.isDebugEnabled()) { - logger.debug("No HttpSession currently exists - new SecurityContext instance " - + "associated with SecurityContextHolder"); - } - - SecurityContextHolder.setContext(generateNewContext()); - } - - // Make the HttpSession null, as we want to ensure we don't keep - // a reference to the HttpSession laying around in case the - // chain.doFilter() invalidates it. - httpSession = null; - - // Proceed with chain - int contextWhenChainProceeded = SecurityContextHolder.getContext().hashCode(); - - try { - 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 an 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 but only if SecurityContext has - // actually changed (see JIRA SEC-37) - if ((httpSession != null) - && (SecurityContextHolder.getContext().hashCode() != contextWhenChainProceeded)) { - httpSession.setAttribute(ACEGI_SECURITY_CONTEXT_KEY, SecurityContextHolder.getContext()); - - if (logger.isDebugEnabled()) { - logger.debug("SecurityContext stored to HttpSession: '" + SecurityContextHolder.getContext() - + "'"); - } - } - - // Remove SecurityContextHolder contents - SecurityContextHolder.clearContext(); - - if (logger.isDebugEnabled()) { - logger.debug("SecurityContextHolder set to new context, as request processing completed"); - } - } - } - } - - public SecurityContext generateNewContext() throws ServletException { - try { - return (SecurityContext) this.context.newInstance(); - } catch (InstantiationException ie) { - throw new ServletException(ie); - } catch (IllegalAccessException iae) { - throw new ServletException(iae); - } - } - - public Class getContext() { - return context; - } - - /** - * Does nothing. We use IoC container lifecycle services instead. - * - * @param filterConfig ignored - * - * @throws ServletException ignored - */ - public void init(FilterConfig filterConfig) throws ServletException {} - - public boolean isAllowSessionCreation() { - return allowSessionCreation; - } - - 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; - } + // ~ 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 + // ================================================================================================ + + 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. 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; + + /** + * Indicates whether the SecurityContext will be cloned from + * the HttpSession. The default is to simply reference (ie + * the default is false). The default may cause issues if + * concurrent threads need to have a different security identity from other + * threads being concurrently processed that share the same + * HttpSession. In most normal environments this does not + * represent an issue, as changes to the security identity in one thread is + * allowed to affect the security identitiy in other threads associated with + * the same HttpSession. For unusual cases where this is not + * permitted, change this value to true and ensure the + * {@link #context} is set to a SecurityContext that + * implements {@link Cloneable} and overrides the clone() + * method. + */ + private boolean cloneFromHttpSession = false; + + public boolean isCloneFromHttpSession() { + return cloneFromHttpSession; + } + + public void setCloneFromHttpSession(boolean cloneFromHttpSession) { + this.cloneFromHttpSession = cloneFromHttpSession; + } + + public HttpSessionContextIntegrationFilter() throws ServletException { + this.contextObject = generateNewContext(); + } + + // ~ Methods + // ======================================================================================================== + + public void afterPropertiesSet() throws Exception { + if ((this.context == null) || (!SecurityContext.class.isAssignableFrom(this.context))) { + throw new IllegalArgumentException("context must be defined and implement SecurityContext " + + "(typically use org.acegisecurity.context.SecurityContextImpl; existing class is " + this.context + + ")"); + } + + if ((forceEagerSessionCreation == true) && (allowSessionCreation == false)) { + throw new IllegalArgumentException( + "If using forceEagerSessionCreation, you must set allowSessionCreation to also be true"); + } + } + + /** + * Does nothing. We use IoC container lifecycle services instead. + */ + public void destroy() { + } + + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, + ServletException { + if ((request != null) && (request.getAttribute(FILTER_APPLIED) != null)) { + // ensure that filter is only applied once per request + chain.doFilter(request, response); + } + else { + if (request != null) { + request.setAttribute(FILTER_APPLIED, Boolean.TRUE); + } + + HttpSession httpSession = null; + boolean httpSessionExistedAtStartOfRequest = false; + + try { + httpSession = ((HttpServletRequest) request).getSession(forceEagerSessionCreation); + } + catch (IllegalStateException ignored) { + } + + if (httpSession != null) { + httpSessionExistedAtStartOfRequest = true; + + Object contextFromSessionObject = httpSession.getAttribute(ACEGI_SECURITY_CONTEXT_KEY); + + if (contextFromSessionObject != null) { + // Clone if required (see SEC-356) + if (cloneFromHttpSession) { + Assert.isInstanceOf(Cloneable.class, contextFromSessionObject, + "Context must implement Clonable and provide a Object.clone() method"); + try { + Method m = contextFromSessionObject.getClass().getMethod("clone", new Class[] {}); + if (!m.isAccessible()) { + m.setAccessible(true); + } + contextFromSessionObject = m.invoke(contextFromSessionObject, new Object[] {}); + } + catch (Exception ex) { + ReflectionUtils.handleReflectionException(ex); + } + } + + if (contextFromSessionObject instanceof SecurityContext) { + if (logger.isDebugEnabled()) { + logger.debug("Obtained from ACEGI_SECURITY_CONTEXT a valid SecurityContext and " + + "set to SecurityContextHolder: '" + contextFromSessionObject + "'"); + } + + SecurityContextHolder.setContext((SecurityContext) contextFromSessionObject); + } + else { + if (logger.isWarnEnabled()) { + logger + .warn("ACEGI_SECURITY_CONTEXT did not contain a SecurityContext but contained: '" + + contextFromSessionObject + + "'; are you improperly modifying the HttpSession directly " + + "(you should always use SecurityContextHolder) or using the HttpSession attribute " + + "reserved for this class? - new SecurityContext instance associated with " + + "SecurityContextHolder"); + } + + SecurityContextHolder.setContext(generateNewContext()); + } + } + else { + if (logger.isDebugEnabled()) { + logger.debug("HttpSession returned null object for ACEGI_SECURITY_CONTEXT - new " + + "SecurityContext instance associated with SecurityContextHolder"); + } + + SecurityContextHolder.setContext(generateNewContext()); + } + } + else { + if (logger.isDebugEnabled()) { + logger.debug("No HttpSession currently exists - new SecurityContext instance " + + "associated with SecurityContextHolder"); + } + + SecurityContextHolder.setContext(generateNewContext()); + } + + // Make the HttpSession null, as we want to ensure we don't keep + // a reference to the HttpSession laying around in case the + // chain.doFilter() invalidates it. + httpSession = null; + + // Proceed with chain + int contextWhenChainProceeded = SecurityContextHolder.getContext().hashCode(); + + try { + 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 an 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 but only if + // SecurityContext has + // actually changed (see JIRA SEC-37) + if ((httpSession != null) + && (SecurityContextHolder.getContext().hashCode() != contextWhenChainProceeded)) { + httpSession.setAttribute(ACEGI_SECURITY_CONTEXT_KEY, SecurityContextHolder.getContext()); + + if (logger.isDebugEnabled()) { + logger.debug("SecurityContext stored to HttpSession: '" + SecurityContextHolder.getContext() + + "'"); + } + } + + // Remove SecurityContextHolder contents + SecurityContextHolder.clearContext(); + + if (logger.isDebugEnabled()) { + logger.debug("SecurityContextHolder set to new context, as request processing completed"); + } + } + } + } + + public SecurityContext generateNewContext() throws ServletException { + try { + return (SecurityContext) this.context.newInstance(); + } + catch (InstantiationException ie) { + throw new ServletException(ie); + } + catch (IllegalAccessException iae) { + throw new ServletException(iae); + } + } + + public Class getContext() { + return context; + } + + /** + * Does nothing. We use IoC container lifecycle services instead. + * + * @param filterConfig ignored + * + * @throws ServletException ignored + */ + public void init(FilterConfig filterConfig) throws ServletException { + } + + public boolean isAllowSessionCreation() { + return allowSessionCreation; + } + + 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; + } }