From fb45db11e9cf99d5710f89133125ee14d59b3e00 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Fri, 5 Jul 2013 12:10:03 -0500 Subject: [PATCH] SEC-2191: Remove AuthenticationManagerBuilder default constructor This ensures that users must choose what ObjectPostProcessor is being used with AuthenticationManagerBuilder. To make things easier for users, we now automatically add an AuthenticationManagerBuilder object that can be used for creating an AuthenticationManager with @Autowired. --- .../AbstractConfiguredSecurityBuilder.java | 34 +++++++++++++++++ .../AuthenticationManagerBuilder.java | 5 ++- .../AuthenticationConfiguration.java | 37 +++++++++++++++++++ .../EnableGlobalMethodSecurity.java | 3 +- .../GlobalMethodSecurityConfiguration.java | 13 ++++++- .../web/configuration/EnableWebSecurity.java | 3 +- .../WebSecurityConfigurerAdapter.java | 16 +++++--- .../config/annotation/BaseSpringSpec.groovy | 2 +- .../AuthenticationManagerBuilderTests.groovy | 4 +- .../BaseAuthenticationConfig.groovy | 9 +---- ...mpleEnableGlobalMethodSecurityTests.groovy | 11 +++--- ...leWebSecurityConfigurerAdapterTests.groovy | 15 ++++---- 12 files changed, 118 insertions(+), 34 deletions(-) create mode 100644 config/src/main/java/org/springframework/security/config/annotation/authentication/configuration/AuthenticationConfiguration.java 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 a3d0202f08..2924eec0ae 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 @@ -23,6 +23,8 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.security.config.annotation.web.builders.WebSecurity; import org.springframework.util.Assert; import org.springframework.web.filter.DelegatingFilterProxy; @@ -51,6 +53,7 @@ import com.google.inject.internal.ImmutableList.Builder; * The type of this builder (that is returned by the base class) */ public abstract class AbstractConfiguredSecurityBuilder> extends AbstractSecurityBuilder { + private final Log logger = LogFactory.getLog(getClass()); private final LinkedHashMap>, List>> configurers = new LinkedHashMap>, List>>(); @@ -95,6 +98,27 @@ public abstract class AbstractConfiguredSecurityBuilder objectPostProcessor) { + super(objectPostProcessor,true); } /** diff --git a/config/src/main/java/org/springframework/security/config/annotation/authentication/configuration/AuthenticationConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/authentication/configuration/AuthenticationConfiguration.java new file mode 100644 index 0000000000..75cc62c27f --- /dev/null +++ b/config/src/main/java/org/springframework/security/config/annotation/authentication/configuration/AuthenticationConfiguration.java @@ -0,0 +1,37 @@ +/* + * Copyright 2002-2013 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.security.config.annotation.authentication.configuration; + +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.security.config.annotation.ObjectPostProcessor; +import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; + +/** + * Exports the authentication {@link Configuration} + * + * @author Rob Winch + * @since 3.2 + * + */ +@Configuration +public class AuthenticationConfiguration { + + @Bean + public AuthenticationManagerBuilder authenticationManagerBuilder(ObjectPostProcessor objectPostProcessor) { + return new AuthenticationManagerBuilder(objectPostProcessor); + } +} diff --git a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/EnableGlobalMethodSecurity.java b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/EnableGlobalMethodSecurity.java index 98cd28d1ea..f8ebc50991 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/EnableGlobalMethodSecurity.java +++ b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/EnableGlobalMethodSecurity.java @@ -23,6 +23,7 @@ import org.springframework.context.annotation.AdviceMode; import org.springframework.context.annotation.Import; import org.springframework.core.Ordered; import org.springframework.security.access.annotation.Secured; +import org.springframework.security.config.annotation.authentication.configuration.AuthenticationConfiguration; import org.springframework.security.config.annotation.configuration.ObjectPostProcessorConfiguration; /** @@ -43,7 +44,7 @@ import org.springframework.security.config.annotation.configuration.ObjectPostPr @Retention(value=java.lang.annotation.RetentionPolicy.RUNTIME) @Target(value={java.lang.annotation.ElementType.TYPE}) @Documented -@Import({GlobalMethodSecuritySelector.class,ObjectPostProcessorConfiguration.class}) +@Import({GlobalMethodSecuritySelector.class,ObjectPostProcessorConfiguration.class,AuthenticationConfiguration.class}) public @interface EnableGlobalMethodSecurity { /** 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 851291af8a..9880f67ad2 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 @@ -21,8 +21,11 @@ import java.util.List; import java.util.Map; import org.aopalliance.intercept.MethodInterceptor; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.aop.framework.ProxyFactoryBean; import org.springframework.aop.target.LazyInitTargetSource; +import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.context.ApplicationContext; @@ -74,6 +77,7 @@ import org.springframework.util.Assert; */ @Configuration public class GlobalMethodSecurityConfiguration implements ImportAware { + private static final Log logger = LogFactory.getLog(GlobalMethodSecurityConfiguration.class); private ApplicationContext context; private ObjectPostProcessor objectPostProcessor = new ObjectPostProcessor() { @Override @@ -82,7 +86,7 @@ public class GlobalMethodSecurityConfiguration implements ImportAware { } }; private AuthenticationManager authenticationManager; - private AuthenticationManagerBuilder auth = new AuthenticationManagerBuilder(); + private AuthenticationManagerBuilder auth = new AuthenticationManagerBuilder(ObjectPostProcessor.QUIESCENT_POSTPROCESSOR); private boolean disableAuthenticationRegistry; private AnnotationAttributes enableMethodSecurity; private MethodSecurityExpressionHandler expressionHandler; @@ -236,6 +240,13 @@ public class GlobalMethodSecurityConfiguration implements ImportAware { if(!disableAuthenticationRegistry) { authenticationManager = auth.build(); } + if(authenticationManager == null) { + try { + authenticationManager = context.getBean(AuthenticationManagerBuilder.class).getOrBuild(); + } catch(NoSuchBeanDefinitionException e) { + logger.debug("Could not obtain the AuthenticationManagerBuilder. This is ok for now, we will try using an AuthenticationManager directly",e); + } + } if(authenticationManager == null) { authenticationManager = lazyBean(AuthenticationManager.class); } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configuration/EnableWebSecurity.java b/config/src/main/java/org/springframework/security/config/annotation/web/configuration/EnableWebSecurity.java index a50d2e73bf..c3d0bbf531 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configuration/EnableWebSecurity.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configuration/EnableWebSecurity.java @@ -20,6 +20,7 @@ import java.lang.annotation.Retention; import java.lang.annotation.Target; import org.springframework.context.annotation.Import; +import org.springframework.security.config.annotation.authentication.configuration.AuthenticationConfiguration; import org.springframework.security.config.annotation.configuration.ObjectPostProcessorConfiguration; import org.springframework.security.config.annotation.web.WebSecurityConfigurer; @@ -76,7 +77,7 @@ import org.springframework.security.config.annotation.web.WebSecurityConfigurer; @Retention(value=java.lang.annotation.RetentionPolicy.RUNTIME) @Target(value={java.lang.annotation.ElementType.TYPE}) @Documented -@Import({WebSecurityConfiguration.class,ObjectPostProcessorConfiguration.class}) +@Import({WebSecurityConfiguration.class,ObjectPostProcessorConfiguration.class,AuthenticationConfiguration.class}) public @interface EnableWebSecurity { /** 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 b5debd2cba..8d8bee0d42 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 @@ -58,8 +58,8 @@ public abstract class WebSecurityConfigurerAdapter implements SecurityConfigurer } }; - private final AuthenticationManagerBuilder authenticationBuilder = new AuthenticationManagerBuilder(); - private final AuthenticationManagerBuilder parentAuthenticationBuilder = new AuthenticationManagerBuilder() { + 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); @@ -194,12 +194,18 @@ public abstract class WebSecurityConfigurerAdapter implements SecurityConfigurer registerAuthentication(parentAuthenticationBuilder); if(disableAuthenticationRegistration) { try { - authenticationManager = context.getBean(AuthenticationManager.class); + authenticationManager = context.getBean(AuthenticationManagerBuilder.class).getOrBuild(); } catch(NoSuchBeanDefinitionException e) { - logger.debug("The AuthenticationManager was not found. This is ok for now as it may not be required.",e); + logger.debug("Could not obtain the AuthenticationManagerBuilder. This is ok for now, we will try using an AuthenticationManager directly",e); + } + if(authenticationManager == null) { + try { + authenticationManager = context.getBean(AuthenticationManager.class); + } catch(NoSuchBeanDefinitionException e) { + logger.debug("The AuthenticationManager was not found. This is ok for now as it may not be required.",e); + } } } else { - authenticationManagerInitialized = true; authenticationManager = parentAuthenticationBuilder.build(); } authenticationManagerInitialized = true; 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 64ab934970..93789459b7 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 @@ -57,7 +57,7 @@ abstract class BaseSpringSpec extends Specification { chain = new MockFilterChain() } - AuthenticationManagerBuilder authenticationBldr = new AuthenticationManagerBuilder().inMemoryAuthentication().and() + AuthenticationManagerBuilder authenticationBldr = new AuthenticationManagerBuilder(ObjectPostProcessor.QUIESCENT_POSTPROCESSOR).inMemoryAuthentication().and() def cleanup() { SecurityContextHolder.clearContext() 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 1f8b23d211..9bcda771d3 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(opp) + AuthenticationManagerBuilder builder = new AuthenticationManagerBuilder(ObjectPostProcessor.QUIESCENT_POSTPROCESSOR).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() + AuthenticationManager am = new AuthenticationManagerBuilder(ObjectPostProcessor.QUIESCENT_POSTPROCESSOR) .authenticationEventPublisher(aep) .inMemoryAuthentication() .and() diff --git a/config/src/test/groovy/org/springframework/security/config/annotation/authentication/BaseAuthenticationConfig.groovy b/config/src/test/groovy/org/springframework/security/config/annotation/authentication/BaseAuthenticationConfig.groovy index 5deb410ca3..f0e82c2efa 100644 --- a/config/src/test/groovy/org/springframework/security/config/annotation/authentication/BaseAuthenticationConfig.groovy +++ b/config/src/test/groovy/org/springframework/security/config/annotation/authentication/BaseAuthenticationConfig.groovy @@ -17,6 +17,7 @@ package org.springframework.security.config.annotation.authentication import java.rmi.registry.Registry; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean import org.springframework.context.annotation.Configuration import org.springframework.security.authentication.AuthenticationManager @@ -33,6 +34,7 @@ import org.springframework.security.core.userdetails.UserDetailsService; */ @Configuration class BaseAuthenticationConfig { + @Autowired protected void registerAuthentication( AuthenticationManagerBuilder auth) throws Exception { auth @@ -40,11 +42,4 @@ class BaseAuthenticationConfig { .withUser("user").password("password").roles("USER").and() .withUser("admin").password("password").roles("USER", "ADMIN").and() } - - @Bean - public AuthenticationManager authenticationManager() { - AuthenticationManagerBuilder registry = new AuthenticationManagerBuilder(); - registerAuthentication(registry); - return registry.build(); - } } diff --git a/config/src/test/groovy/org/springframework/security/config/annotation/method/configuration/SampleEnableGlobalMethodSecurityTests.groovy b/config/src/test/groovy/org/springframework/security/config/annotation/method/configuration/SampleEnableGlobalMethodSecurityTests.groovy index 01e61314cd..49e0025c14 100644 --- a/config/src/test/groovy/org/springframework/security/config/annotation/method/configuration/SampleEnableGlobalMethodSecurityTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/annotation/method/configuration/SampleEnableGlobalMethodSecurityTests.groovy @@ -15,6 +15,7 @@ */ package org.springframework.security.config.annotation.method.configuration +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean import org.springframework.context.annotation.Configuration import org.springframework.security.access.AccessDeniedException @@ -65,14 +66,12 @@ public class SampleEnableGlobalMethodSecurityTests extends BaseSpringSpec { return new MethodSecurityServiceImpl() } - @Bean - public AuthenticationManager authenticationManager() throws Exception { - return new AuthenticationManagerBuilder() + @Autowired + public void registerAuthentication(AuthenticationManagerBuilder auth) throws Exception { + auth .inMemoryAuthentication() .withUser("user").password("password").roles("USER").and() - .withUser("admin").password("password").roles("USER", "ADMIN").and() - .and() - .build(); + .withUser("admin").password("password").roles("USER", "ADMIN"); } } diff --git a/config/src/test/groovy/org/springframework/security/config/annotation/web/SampleWebSecurityConfigurerAdapterTests.groovy b/config/src/test/groovy/org/springframework/security/config/annotation/web/SampleWebSecurityConfigurerAdapterTests.groovy index 9a83dd68de..f76d618801 100644 --- a/config/src/test/groovy/org/springframework/security/config/annotation/web/SampleWebSecurityConfigurerAdapterTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/annotation/web/SampleWebSecurityConfigurerAdapterTests.groovy @@ -15,6 +15,7 @@ */ package org.springframework.security.config.annotation.web +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean import org.springframework.context.annotation.Configuration import org.springframework.core.annotation.Order @@ -272,14 +273,12 @@ public class SampleWebSecurityConfigurerAdapterTests extends BaseWebSpecuritySpe @Configuration @EnableWebSecurity public static class SampleMultiHttpSecurityConfig { - @Bean - public AuthenticationManager authenticationManager() { - return new AuthenticationManagerBuilder() - .inMemoryAuthentication() - .withUser("user").password("password").roles("USER").and() - .withUser("admin").password("password").roles("USER", "ADMIN").and() - .and() - .build(); + @Autowired + public void registerAuthentication(AuthenticationManagerBuilder auth) { + auth + .inMemoryAuthentication() + .withUser("user").password("password").roles("USER").and() + .withUser("admin").password("password").roles("USER", "ADMIN"); } @Configuration