From 48dce501ce2f1af326dd84fab81ad564783328fa Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Fri, 12 Dec 2008 14:27:23 +0000 Subject: [PATCH] SEC-942: Added createEmptyContext() method to SecurityContextHolderStrategy and SecurityContextHolder to encapsulate the context implemetentation in one place. HttpSessionSecurityContextRepository calls this method when it needs a new context to store in the session. --- .../GlobalSecurityContextHolderStrategy.java | 11 ++++++++--- .../HttpSessionSecurityContextRepository.java | 19 +++++++++++++------ ...eadLocalSecurityContextHolderStrategy.java | 6 +++++- .../context/SecurityContextHolder.java | 7 +++++++ .../SecurityContextHolderStrategy.java | 12 +++++++++--- .../SecurityContextPersistenceFilter.java | 2 +- ...eadLocalSecurityContextHolderStrategy.java | 9 ++++++--- 7 files changed, 49 insertions(+), 17 deletions(-) diff --git a/core/src/main/java/org/springframework/security/context/GlobalSecurityContextHolderStrategy.java b/core/src/main/java/org/springframework/security/context/GlobalSecurityContextHolderStrategy.java index 53541e1752..96311c34de 100644 --- a/core/src/main/java/org/springframework/security/context/GlobalSecurityContextHolderStrategy.java +++ b/core/src/main/java/org/springframework/security/context/GlobalSecurityContextHolderStrategy.java @@ -19,9 +19,10 @@ import org.springframework.util.Assert; /** - * A static field-based implementation of {@link - * org.springframework.security.context.SecurityContextHolderStrategy}.

This means that all instances in the JVM share the - * same SecurityContext. This is generally useful with rich clients, such as Swing.

+ * A static field-based implementation of {@link SecurityContextHolderStrategy}. + *

+ * This means that all instances in the JVM share the + * same SecurityContext. This is generally useful with rich clients, such as Swing. * * @author Ben Alex * @version $Id$ @@ -49,4 +50,8 @@ public class GlobalSecurityContextHolderStrategy implements SecurityContextHolde Assert.notNull(context, "Only non-null SecurityContext instances are permitted"); contextHolder = context; } + + public SecurityContext createEmptyContext() { + return new SecurityContextImpl(); + } } 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 e2280a4880..2551f7fe69 100644 --- a/core/src/main/java/org/springframework/security/context/HttpSessionSecurityContextRepository.java +++ b/core/src/main/java/org/springframework/security/context/HttpSessionSecurityContextRepository.java @@ -180,19 +180,26 @@ public class HttpSessionSecurityContextRepository implements SecurityContextRepo } /** - * By default, returns an instance of {@link SecurityContextImpl}. - * If a custom SecurityContext implementation is in use (i.e. the securityContextClass property - * is set), it will attempt to invoke the no-args constructor on the supplied class instead and return the created - * instance. + * By default, calls {@link SecurityContextHolder#createEmptyContext()} to obtain a new context (there should be + * no context present in the holder when this method is called). Using this approach the context creation + * strategy is decided by the {@link SecurityContextHolderStrategy} in use. The default implementations + * will return a new SecurityContextImpl. + *

+ * An alternative way of customizing the SecurityContext implementation is by setting the + * securityContextClass property. In this case, the method will attempt to invoke the no-args + * constructor on the supplied class instead and return the created instance. * * @return a new SecurityContext instance. Never null. */ SecurityContext generateNewContext() { + SecurityContext context = null; + if (securityContextClass == null) { - return new SecurityContextImpl(); + context = SecurityContextHolder.createEmptyContext(); + + return context; } - SecurityContext context = null; try { context = securityContextClass.newInstance(); } catch (Exception e) { diff --git a/core/src/main/java/org/springframework/security/context/InheritableThreadLocalSecurityContextHolderStrategy.java b/core/src/main/java/org/springframework/security/context/InheritableThreadLocalSecurityContextHolderStrategy.java index 6cf50d7acb..cbb90d0a42 100644 --- a/core/src/main/java/org/springframework/security/context/InheritableThreadLocalSecurityContextHolderStrategy.java +++ b/core/src/main/java/org/springframework/security/context/InheritableThreadLocalSecurityContextHolderStrategy.java @@ -26,7 +26,7 @@ import org.springframework.util.Assert; * @version $Id$ * * @see java.lang.ThreadLocal - * @see org.springframework.security.context.HttpSessionContextIntegrationFilter + * @see org.springframework.security.context.SecurityContextPersistenceFilter */ public class InheritableThreadLocalSecurityContextHolderStrategy implements SecurityContextHolderStrategy { //~ Static fields/initializers ===================================================================================== @@ -51,4 +51,8 @@ public class InheritableThreadLocalSecurityContextHolderStrategy implements Secu Assert.notNull(context, "Only non-null SecurityContext instances are permitted"); contextHolder.set(context); } + + public SecurityContext createEmptyContext() { + return new SecurityContextImpl(); + } } diff --git a/core/src/main/java/org/springframework/security/context/SecurityContextHolder.java b/core/src/main/java/org/springframework/security/context/SecurityContextHolder.java index 7aa741e8ad..67673f0b1b 100644 --- a/core/src/main/java/org/springframework/security/context/SecurityContextHolder.java +++ b/core/src/main/java/org/springframework/security/context/SecurityContextHolder.java @@ -135,6 +135,13 @@ public class SecurityContextHolder { initialize(); } + /** + * Delegates the creation of a new, empty context to the configured strategy. + */ + static SecurityContext createEmptyContext() { + return strategy.createEmptyContext(); + } + public String toString() { return "SecurityContextHolder[strategy='" + strategyName + "'; initializeCount=" + initializeCount + "]"; } diff --git a/core/src/main/java/org/springframework/security/context/SecurityContextHolderStrategy.java b/core/src/main/java/org/springframework/security/context/SecurityContextHolderStrategy.java index a26b6a6d09..6ac1110a52 100644 --- a/core/src/main/java/org/springframework/security/context/SecurityContextHolderStrategy.java +++ b/core/src/main/java/org/springframework/security/context/SecurityContextHolderStrategy.java @@ -19,9 +19,7 @@ package org.springframework.security.context; * A strategy for storing security context information against a thread. * *

- * The preferred strategy is loaded by {@link - * org.springframework.security.context.SecurityContextHolder}. - *

+ * The preferred strategy is loaded by {@link SecurityContextHolder}. * * @author Ben Alex * @version $Id$ @@ -48,4 +46,12 @@ public interface SecurityContextHolderStrategy { * null has been passed and throw an IllegalArgumentException in such cases) */ void setContext(SecurityContext context); + + /** + * Creates a new, empty context implementation, for use by SecurityContextRepository implementations, + * when creating a new context for the first time. + * + * @return the empty context. + */ + SecurityContext createEmptyContext(); } diff --git a/core/src/main/java/org/springframework/security/context/SecurityContextPersistenceFilter.java b/core/src/main/java/org/springframework/security/context/SecurityContextPersistenceFilter.java index a60b917045..f66d681b38 100644 --- a/core/src/main/java/org/springframework/security/context/SecurityContextPersistenceFilter.java +++ b/core/src/main/java/org/springframework/security/context/SecurityContextPersistenceFilter.java @@ -61,9 +61,9 @@ public class SecurityContextPersistenceFilter extends SpringSecurityFilter { } HttpRequestResponseHolder holder = new HttpRequestResponseHolder(request, response); + SecurityContext contextBeforeChainExecution = repo.loadContext(holder); try { - SecurityContext contextBeforeChainExecution = repo.loadContext(holder); SecurityContextHolder.setContext(contextBeforeChainExecution); chain.doFilter(holder.getRequest(), holder.getResponse()); diff --git a/core/src/main/java/org/springframework/security/context/ThreadLocalSecurityContextHolderStrategy.java b/core/src/main/java/org/springframework/security/context/ThreadLocalSecurityContextHolderStrategy.java index a4ef580798..a6a7e2954f 100644 --- a/core/src/main/java/org/springframework/security/context/ThreadLocalSecurityContextHolderStrategy.java +++ b/core/src/main/java/org/springframework/security/context/ThreadLocalSecurityContextHolderStrategy.java @@ -19,14 +19,13 @@ import org.springframework.util.Assert; /** - * A ThreadLocal-based implementation of {@link - * org.springframework.security.context.SecurityContextHolderStrategy}. + * A ThreadLocal-based implementation of {@link SecurityContextHolderStrategy}. * * @author Ben Alex * @version $Id$ * * @see java.lang.ThreadLocal - * @see org.springframework.security.context.HttpSessionContextIntegrationFilter + * @see org.springframework.security.context.SecurityContextPersistenceFilter */ public class ThreadLocalSecurityContextHolderStrategy implements SecurityContextHolderStrategy { //~ Static fields/initializers ===================================================================================== @@ -51,4 +50,8 @@ public class ThreadLocalSecurityContextHolderStrategy implements SecurityContext Assert.notNull(context, "Only non-null SecurityContext instances are permitted"); contextHolder.set(context); } + + public SecurityContext createEmptyContext() { + return new SecurityContextImpl(); + } }