From 5f290ba10f3a048db4add36c5055e0e2fb4058ba Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Fri, 18 Oct 2013 14:31:13 -0500 Subject: [PATCH] SEC-2371: Remove ObjectPostProcessor.QUIESENT_POSTPROCESSOR --- .../AbstractConfiguredSecurityBuilder.java | 7 ----- .../annotation/ObjectPostProcessor.java | 11 +------- .../GlobalMethodSecurityConfiguration.java | 4 +-- .../annotation/web/builders/WebSecurity.java | 5 +++- .../WebSecurityConfiguration.java | 23 +++++++++------ .../WebSecurityConfigurerAdapter.java | 28 +++++++++++-------- .../config/annotation/BaseSpringSpec.groovy | 22 +++++++++++++-- .../AuthenticationManagerBuilderTests.groovy | 4 +-- ...tractConfiguredSecurityBuilderTests.groovy | 22 ++++++++------- 9 files changed, 70 insertions(+), 56 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/annotation/AbstractConfiguredSecurityBuilder.java b/config/src/main/java/org/springframework/security/config/annotation/AbstractConfiguredSecurityBuilder.java index 2924eec0ae..e2654de454 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/AbstractConfiguredSecurityBuilder.java +++ b/config/src/main/java/org/springframework/security/config/annotation/AbstractConfiguredSecurityBuilder.java @@ -66,13 +66,6 @@ public abstract class AbstractConfiguredSecurityBuilder objectPostProcessor; - /** - * Creates a new instance without post processing - */ - protected AbstractConfiguredSecurityBuilder() { - this(ObjectPostProcessor.QUIESCENT_POSTPROCESSOR); - } - /*** * Creates a new instance with the provided {@link ObjectPostProcessor}. * This post processor must support Object since there are many types of diff --git a/config/src/main/java/org/springframework/security/config/annotation/ObjectPostProcessor.java b/config/src/main/java/org/springframework/security/config/annotation/ObjectPostProcessor.java index be2c7283e6..02189fcac7 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/ObjectPostProcessor.java +++ b/config/src/main/java/org/springframework/security/config/annotation/ObjectPostProcessor.java @@ -39,13 +39,4 @@ public interface ObjectPostProcessor { * @return the initialized version of the object */ O postProcess(O object); - - /** - * A do nothing implementation of the {@link ObjectPostProcessor} - */ - ObjectPostProcessor QUIESCENT_POSTPROCESSOR = new ObjectPostProcessor() { - public T postProcess(T object) { - return object; - } - }; -} +} \ No newline at end of file diff --git a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.java index 12d63ad5c5..0eccb8d978 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.java +++ b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.java @@ -89,7 +89,7 @@ public class GlobalMethodSecurityConfiguration implements ImportAware { }; private DefaultMethodSecurityExpressionHandler defaultMethodExpressionHandler = new DefaultMethodSecurityExpressionHandler(); private AuthenticationManager authenticationManager; - private AuthenticationManagerBuilder auth = new AuthenticationManagerBuilder(ObjectPostProcessor.QUIESCENT_POSTPROCESSOR); + private AuthenticationManagerBuilder auth; private boolean disableAuthenticationRegistry; private AnnotationAttributes enableMethodSecurity; private MethodSecurityExpressionHandler expressionHandler; @@ -245,8 +245,8 @@ public class GlobalMethodSecurityConfiguration implements ImportAware { protected AuthenticationManager authenticationManager() throws Exception { if(authenticationManager == null) { DefaultAuthenticationEventPublisher eventPublisher = objectPostProcessor.postProcess(new DefaultAuthenticationEventPublisher()); + auth = new AuthenticationManagerBuilder(objectPostProcessor); auth.authenticationEventPublisher(eventPublisher); - auth.objectPostProcessor(objectPostProcessor); configure(auth); if(!disableAuthenticationRegistry) { authenticationManager = auth.build(); diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurity.java b/config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurity.java index 19d083dbdd..c7ddd2cca4 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurity.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurity.java @@ -28,6 +28,7 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; import org.springframework.security.access.expression.SecurityExpressionHandler; import org.springframework.security.config.annotation.AbstractConfiguredSecurityBuilder; +import org.springframework.security.config.annotation.ObjectPostProcessor; import org.springframework.security.config.annotation.SecurityBuilder; import org.springframework.security.config.annotation.web.AbstractRequestMatcherRegistry; import org.springframework.security.config.annotation.web.WebSecurityConfigurer; @@ -101,9 +102,11 @@ public final class WebSecurity extends /** * Creates a new instance + * @param objectPostProcessor the {@link ObjectPostProcessor} to use * @see WebSecurityConfiguration */ - public WebSecurity() { + public WebSecurity(ObjectPostProcessor objectPostProcessor) { + super(objectPostProcessor); } /** diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebSecurityConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebSecurityConfiguration.java index f7a4815894..8712645e7b 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebSecurityConfiguration.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebSecurityConfiguration.java @@ -62,7 +62,9 @@ import org.springframework.util.ClassUtils; */ @Configuration public class WebSecurityConfiguration implements ImportAware, BeanClassLoaderAware { - private final WebSecurity webSecurity = new WebSecurity(); + private WebSecurity webSecurity; + + private Boolean debugEnabled; private List> webSecurityConfigurers; @@ -102,12 +104,18 @@ public class WebSecurityConfiguration implements ImportAware, BeanClassLoaderAwa /** * Sets the {@code } instances used to create the web configuration. * + * @param objectPostProcessor the {@link ObjectPostProcessor} used to create a {@link WebSecurity} instance * @param webSecurityConfigurers the {@code } instances used to create the web configuration * @throws Exception */ @Autowired(required = false) - public void setFilterChainProxySecurityConfigurer( + public void setFilterChainProxySecurityConfigurer(ObjectPostProcessor objectPostProcessor, List> webSecurityConfigurers) throws Exception { + webSecurity = objectPostProcessor.postProcess(new WebSecurity(objectPostProcessor)); + if(debugEnabled != null) { + webSecurity.debug(debugEnabled); + } + Collections.sort(webSecurityConfigurers, AnnotationAwareOrderComparator.INSTANCE); Integer previousOrder = null; @@ -175,8 +183,10 @@ public class WebSecurityConfiguration implements ImportAware, BeanClassLoaderAwa enableWebSecurityAttrs = AnnotationAttributes.fromMap(enableWebSecurityAttrMap); } } - boolean debugEnabled = enableWebSecurityAttrs.getBoolean("debug"); - this.webSecurity.debug(debugEnabled); + debugEnabled = enableWebSecurityAttrs.getBoolean("debug"); + if(webSecurity != null) { + webSecurity.debug(debugEnabled); + } } /* (non-Javadoc) @@ -185,9 +195,4 @@ public class WebSecurityConfiguration implements ImportAware, BeanClassLoaderAwa public void setBeanClassLoader(ClassLoader classLoader) { this.beanClassLoader = classLoader; } - - @Autowired - public void setObjectPostProcessor(ObjectPostProcessor objectPostProcessor) { - objectPostProcessor.postProcess(webSecurity); - } } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebSecurityConfigurerAdapter.java b/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebSecurityConfigurerAdapter.java index 8d4d0f686d..15c6ce52cb 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebSecurityConfigurerAdapter.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebSecurityConfigurerAdapter.java @@ -45,6 +45,7 @@ import org.springframework.security.core.userdetails.UserDetailsService; import org.springframework.security.core.userdetails.UsernameNotFoundException; import org.springframework.security.web.access.intercept.FilterSecurityInterceptor; import org.springframework.security.web.context.request.async.WebAsyncManagerIntegrationFilter; +import org.springframework.util.Assert; import org.springframework.web.accept.ContentNegotiationStrategy; import org.springframework.web.accept.HeaderContentNegotiationStrategy; @@ -69,15 +70,8 @@ public abstract class WebSecurityConfigurerAdapter implements SecurityConfigurer } }; - private final AuthenticationManagerBuilder authenticationBuilder = new AuthenticationManagerBuilder(ObjectPostProcessor.QUIESCENT_POSTPROCESSOR); - private final AuthenticationManagerBuilder parentAuthenticationBuilder = new AuthenticationManagerBuilder(ObjectPostProcessor.QUIESCENT_POSTPROCESSOR) { - @Override - public AuthenticationManagerBuilder eraseCredentials(boolean eraseCredentials) { - authenticationBuilder.eraseCredentials(eraseCredentials); - return super.eraseCredentials(eraseCredentials); - } - - }; + private AuthenticationManagerBuilder authenticationBuilder; + private AuthenticationManagerBuilder parentAuthenticationBuilder; private boolean disableAuthenticationRegistration; private boolean authenticationManagerInitialized; private AuthenticationManager authenticationManager; @@ -168,9 +162,6 @@ public abstract class WebSecurityConfigurerAdapter implements SecurityConfigurer return http; } - authenticationBuilder.objectPostProcessor(objectPostProcessor); - parentAuthenticationBuilder.objectPostProcessor(objectPostProcessor); - DefaultAuthenticationEventPublisher eventPublisher = objectPostProcessor.postProcess(new DefaultAuthenticationEventPublisher()); parentAuthenticationBuilder.authenticationEventPublisher(eventPublisher); @@ -355,6 +346,16 @@ public abstract class WebSecurityConfigurerAdapter implements SecurityConfigurer @Autowired(required=false) public void setObjectPostProcessor(ObjectPostProcessor objectPostProcessor) { this.objectPostProcessor = objectPostProcessor; + + authenticationBuilder = new AuthenticationManagerBuilder(objectPostProcessor); + parentAuthenticationBuilder = new AuthenticationManagerBuilder(objectPostProcessor) { + @Override + public AuthenticationManagerBuilder eraseCredentials(boolean eraseCredentials) { + authenticationBuilder.eraseCredentials(eraseCredentials); + return super.eraseCredentials(eraseCredentials); + } + + }; } @@ -372,6 +373,9 @@ public abstract class WebSecurityConfigurerAdapter implements SecurityConfigurer private final Object delegateMonitor = new Object(); UserDetailsServiceDelegator(List delegateBuilders) { + if(delegateBuilders.contains(null)) { + throw new IllegalArgumentException("delegateBuilders cannot contain null values. Got " + delegateBuilders); + } this.delegateBuilders = delegateBuilders; } diff --git a/config/src/test/groovy/org/springframework/security/config/annotation/BaseSpringSpec.groovy b/config/src/test/groovy/org/springframework/security/config/annotation/BaseSpringSpec.groovy index f4304eb58a..0a3068f269 100644 --- a/config/src/test/groovy/org/springframework/security/config/annotation/BaseSpringSpec.groovy +++ b/config/src/test/groovy/org/springframework/security/config/annotation/BaseSpringSpec.groovy @@ -20,6 +20,7 @@ import javax.servlet.Filter import org.springframework.beans.factory.NoSuchBeanDefinitionException import org.springframework.context.ConfigurableApplicationContext import org.springframework.context.annotation.AnnotationConfigApplicationContext +import org.springframework.context.annotation.Configuration; import org.springframework.mock.web.MockFilterChain import org.springframework.mock.web.MockHttpServletRequest import org.springframework.mock.web.MockHttpServletResponse @@ -27,6 +28,9 @@ import org.springframework.security.authentication.AuthenticationManager import org.springframework.security.authentication.AuthenticationProvider import org.springframework.security.authentication.UsernamePasswordAuthenticationToken import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder +import org.springframework.security.config.annotation.authentication.configuration.AuthenticationConfiguration; +import org.springframework.security.config.annotation.configuration.AutowireBeanFactoryObjectPostProcessor; +import org.springframework.security.config.annotation.configuration.ObjectPostProcessorConfiguration; import org.springframework.security.core.Authentication import org.springframework.security.core.authority.AuthorityUtils import org.springframework.security.core.context.SecurityContextHolder @@ -49,13 +53,17 @@ import spock.lang.Specification abstract class BaseSpringSpec extends Specification { @AutoCleanup ConfigurableApplicationContext context + @AutoCleanup + ConfigurableApplicationContext oppContext MockHttpServletRequest request MockHttpServletResponse response MockFilterChain chain CsrfToken csrfToken + AuthenticationManagerBuilder authenticationBldr def setup() { + authenticationBldr = createAuthenticationManagerBuilder() setupWeb(null) } @@ -75,8 +83,6 @@ abstract class BaseSpringSpec extends Specification { req.setParameter(csrfToken.parameterName, csrfToken.token) } - AuthenticationManagerBuilder authenticationBldr = new AuthenticationManagerBuilder(ObjectPostProcessor.QUIESCENT_POSTPROCESSOR).inMemoryAuthentication().and() - def cleanup() { SecurityContextHolder.clearContext() } @@ -149,4 +155,14 @@ abstract class BaseSpringSpec extends Specification { repo.loadContext(requestResponseHolder) repo.saveContext(new SecurityContextImpl(authentication:auth), requestResponseHolder.request, requestResponseHolder.response) } -} + + def createAuthenticationManagerBuilder() { + oppContext = new AnnotationConfigApplicationContext(ObjectPostProcessorConfiguration, AuthenticationConfiguration) + AuthenticationManagerBuilder auth = new AuthenticationManagerBuilder(objectPostProcessor) + auth.inMemoryAuthentication().and() + } + + def getObjectPostProcessor() { + oppContext.getBean(ObjectPostProcessor) + } +} \ No newline at end of file diff --git a/config/src/test/groovy/org/springframework/security/config/annotation/authentication/AuthenticationManagerBuilderTests.groovy b/config/src/test/groovy/org/springframework/security/config/annotation/authentication/AuthenticationManagerBuilderTests.groovy index c030bf4600..d61f1e044f 100644 --- a/config/src/test/groovy/org/springframework/security/config/annotation/authentication/AuthenticationManagerBuilderTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/annotation/authentication/AuthenticationManagerBuilderTests.groovy @@ -38,7 +38,7 @@ class AuthenticationManagerBuilderTests extends BaseSpringSpec { setup: ObjectPostProcessor opp = Mock() AuthenticationProvider provider = Mock() - AuthenticationManagerBuilder builder = new AuthenticationManagerBuilder(ObjectPostProcessor.QUIESCENT_POSTPROCESSOR).objectPostProcessor(opp) + AuthenticationManagerBuilder builder = new AuthenticationManagerBuilder(objectPostProcessor).objectPostProcessor(opp) when: "Adding an AuthenticationProvider" builder.authenticationProvider(provider) builder.build() @@ -51,7 +51,7 @@ class AuthenticationManagerBuilderTests extends BaseSpringSpec { setup: AuthenticationEventPublisher aep = Mock() when: - AuthenticationManager am = new AuthenticationManagerBuilder(ObjectPostProcessor.QUIESCENT_POSTPROCESSOR) + AuthenticationManager am = new AuthenticationManagerBuilder(objectPostProcessor) .authenticationEventPublisher(aep) .inMemoryAuthentication() .and() diff --git a/config/src/test/groovy/org/springframework/security/config/annotation/web/AbstractConfiguredSecurityBuilderTests.groovy b/config/src/test/groovy/org/springframework/security/config/annotation/web/AbstractConfiguredSecurityBuilderTests.groovy index 1f15282971..1da0539680 100644 --- a/config/src/test/groovy/org/springframework/security/config/annotation/web/AbstractConfiguredSecurityBuilderTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/annotation/web/AbstractConfiguredSecurityBuilderTests.groovy @@ -16,6 +16,7 @@ package org.springframework.security.config.annotation.web import org.springframework.security.config.annotation.AbstractConfiguredSecurityBuilder +import org.springframework.security.config.annotation.BaseSpringSpec import org.springframework.security.config.annotation.ObjectPostProcessor; import org.springframework.security.config.annotation.SecurityConfigurer import org.springframework.security.config.annotation.SecurityConfigurerAdapter @@ -27,9 +28,13 @@ import spock.lang.Specification * @author Rob Winch * */ -class AbstractConfiguredSecurityBuilderTests extends Specification { +class AbstractConfiguredSecurityBuilderTests extends BaseSpringSpec { - ConcreteAbstractConfiguredBuilder builder = new ConcreteAbstractConfiguredBuilder() + ConcreteAbstractConfiguredBuilder builder + + def setup() { + builder = new ConcreteAbstractConfiguredBuilder(objectPostProcessor) + } def "Null ObjectPostProcessor rejected"() { when: @@ -86,7 +91,7 @@ class AbstractConfiguredSecurityBuilderTests extends Specification { def "getConfigurer with multi fails"() { setup: - ConcreteAbstractConfiguredBuilder builder = new ConcreteAbstractConfiguredBuilder(ObjectPostProcessor.QUIESCENT_POSTPROCESSOR, true) + ConcreteAbstractConfiguredBuilder builder = new ConcreteAbstractConfiguredBuilder(objectPostProcessor, true) builder.apply(new DelegateConfigurer()) builder.apply(new DelegateConfigurer()) when: @@ -97,7 +102,7 @@ class AbstractConfiguredSecurityBuilderTests extends Specification { def "removeConfigurer with multi fails"() { setup: - ConcreteAbstractConfiguredBuilder builder = new ConcreteAbstractConfiguredBuilder(ObjectPostProcessor.QUIESCENT_POSTPROCESSOR, true) + ConcreteAbstractConfiguredBuilder builder = new ConcreteAbstractConfiguredBuilder(objectPostProcessor, true) builder.apply(new DelegateConfigurer()) builder.apply(new DelegateConfigurer()) when: @@ -110,7 +115,7 @@ class AbstractConfiguredSecurityBuilderTests extends Specification { setup: DelegateConfigurer c1 = new DelegateConfigurer() DelegateConfigurer c2 = new DelegateConfigurer() - ConcreteAbstractConfiguredBuilder builder = new ConcreteAbstractConfiguredBuilder(ObjectPostProcessor.QUIESCENT_POSTPROCESSOR, true) + ConcreteAbstractConfiguredBuilder builder = new ConcreteAbstractConfiguredBuilder(objectPostProcessor, true) builder.apply(c1) builder.apply(c2) when: @@ -126,7 +131,7 @@ class AbstractConfiguredSecurityBuilderTests extends Specification { setup: DelegateConfigurer c1 = new DelegateConfigurer() DelegateConfigurer c2 = new DelegateConfigurer() - ConcreteAbstractConfiguredBuilder builder = new ConcreteAbstractConfiguredBuilder(ObjectPostProcessor.QUIESCENT_POSTPROCESSOR, true) + ConcreteAbstractConfiguredBuilder builder = new ConcreteAbstractConfiguredBuilder(objectPostProcessor, true) builder.apply(c1) builder.apply(c2) when: @@ -150,10 +155,7 @@ class AbstractConfiguredSecurityBuilderTests extends Specification { private static class ConcreteConfigurer extends SecurityConfigurerAdapter { } - private static class ConcreteAbstractConfiguredBuilder extends AbstractConfiguredSecurityBuilder { - - public ConcreteAbstractConfiguredBuilder() { - } + private class ConcreteAbstractConfiguredBuilder extends AbstractConfiguredSecurityBuilder { public ConcreteAbstractConfiguredBuilder(ObjectPostProcessor objectPostProcessor) { super(objectPostProcessor);