From 974105ed193c30dc663be3657a38d1904d7b5435 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Mon, 10 Mar 2014 14:02:02 -0500 Subject: [PATCH] SEC-2515: Detect object cycle for AuthenticationManager configuration --- .../AuthenticationConfiguration.java | 3 +- .../WebSecurityConfigurerAdapter.java | 41 ++++++++--- .../web/configuration/Sec2515Tests.groovy | 72 +++++++++++++++++++ 3 files changed, 104 insertions(+), 12 deletions(-) create mode 100644 config/src/test/groovy/org/springframework/security/config/annotation/web/configuration/Sec2515Tests.groovy 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 index 58c4846445..b83345b3a0 100644 --- 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 @@ -114,7 +114,7 @@ public class AuthenticationConfiguration { lazyTargetSource.setBeanFactory(applicationContext); ProxyFactoryBean proxyFactory = new ProxyFactoryBean(); proxyFactory.setTargetSource(lazyTargetSource); - proxyFactory.setInterfaces(new Class[] { interfaceName }); + proxyFactory.setInterfaces(new Class[] { interfaceName, LazyBean.class }); return (T) proxyFactory.getObject(); } @@ -122,6 +122,7 @@ public class AuthenticationConfiguration { return lazyBean(AuthenticationManager.class); } + private interface LazyBean {} private static class EnableGlobalAuthenticationAutowiredConfigurer extends GlobalAuthenticationConfigurerAdapter { private final ApplicationContext context; 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 865d8e63b8..79f3ee822b 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 @@ -16,11 +16,13 @@ package org.springframework.security.config.annotation.web.configuration; +import java.lang.reflect.Field; import java.util.Arrays; import java.util.List; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.beans.FatalBeanException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationContext; import org.springframework.core.annotation.Order; @@ -44,6 +46,7 @@ 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.util.ReflectionUtils; import org.springframework.web.accept.ContentNegotiationStrategy; import org.springframework.web.accept.HeaderContentNegotiationStrategy; @@ -71,8 +74,8 @@ public abstract class WebSecurityConfigurerAdapter implements WebSecurityConfigu private AuthenticationConfiguration authenticationConfiguration; private AuthenticationManagerBuilder authenticationBuilder; - private AuthenticationManagerBuilder parentAuthenticationBuilder; - private boolean disableAuthenticationRegistration; + private AuthenticationManagerBuilder localConfigureAuthenticationBldr; + private boolean disableLocalConfigureAuthenticationBldr; private boolean authenticationManagerInitialized; private AuthenticationManager authenticationManager; private AuthenticationTrustResolver trustResolver = new AuthenticationTrustResolverImpl(); @@ -148,7 +151,7 @@ public abstract class WebSecurityConfigurerAdapter implements WebSecurityConfigu * @throws Exception */ protected void configure(AuthenticationManagerBuilder auth) throws Exception { - this.disableAuthenticationRegistration = true; + this.disableLocalConfigureAuthenticationBldr = true; } /** @@ -163,11 +166,11 @@ public abstract class WebSecurityConfigurerAdapter implements WebSecurityConfigu } DefaultAuthenticationEventPublisher eventPublisher = objectPostProcessor.postProcess(new DefaultAuthenticationEventPublisher()); - parentAuthenticationBuilder.authenticationEventPublisher(eventPublisher); + localConfigureAuthenticationBldr.authenticationEventPublisher(eventPublisher); AuthenticationManager authenticationManager = authenticationManager(); authenticationBuilder.parentAuthenticationManager(authenticationManager); - http = new HttpSecurity(objectPostProcessor,authenticationBuilder, parentAuthenticationBuilder.getSharedObjects()); + http = new HttpSecurity(objectPostProcessor,authenticationBuilder, localConfigureAuthenticationBldr.getSharedObjects()); http.setSharedObject(UserDetailsService.class, userDetailsService()); http.setSharedObject(ApplicationContext.class, context); http.setSharedObject(ContentNegotiationStrategy.class, contentNegotiationStrategy); @@ -221,11 +224,11 @@ public abstract class WebSecurityConfigurerAdapter implements WebSecurityConfigu */ protected AuthenticationManager authenticationManager() throws Exception { if(!authenticationManagerInitialized) { - configure(parentAuthenticationBuilder); - if(disableAuthenticationRegistration) { + configure(localConfigureAuthenticationBldr); + if(disableLocalConfigureAuthenticationBldr) { authenticationManager = authenticationConfiguration.getAuthenticationManager(); } else { - authenticationManager = parentAuthenticationBuilder.build(); + authenticationManager = localConfigureAuthenticationBldr.build(); } authenticationManagerInitialized = true; } @@ -253,7 +256,7 @@ public abstract class WebSecurityConfigurerAdapter implements WebSecurityConfigu */ public UserDetailsService userDetailsServiceBean() throws Exception { AuthenticationManagerBuilder globalAuthBuilder = context.getBean(AuthenticationManagerBuilder.class); - return new UserDetailsServiceDelegator(Arrays.asList(parentAuthenticationBuilder, globalAuthBuilder)); + return new UserDetailsServiceDelegator(Arrays.asList(localConfigureAuthenticationBldr, globalAuthBuilder)); } /** @@ -266,7 +269,7 @@ public abstract class WebSecurityConfigurerAdapter implements WebSecurityConfigu */ protected UserDetailsService userDetailsService() { AuthenticationManagerBuilder globalAuthBuilder = context.getBean(AuthenticationManagerBuilder.class); - return new UserDetailsServiceDelegator(Arrays.asList(parentAuthenticationBuilder, globalAuthBuilder)); + return new UserDetailsServiceDelegator(Arrays.asList(localConfigureAuthenticationBldr, globalAuthBuilder)); } public void init(final WebSecurity web) throws Exception { @@ -337,7 +340,7 @@ public abstract class WebSecurityConfigurerAdapter implements WebSecurityConfigu this.objectPostProcessor = objectPostProcessor; authenticationBuilder = new AuthenticationManagerBuilder(objectPostProcessor); - parentAuthenticationBuilder = new AuthenticationManagerBuilder(objectPostProcessor) { + localConfigureAuthenticationBldr = new AuthenticationManagerBuilder(objectPostProcessor) { @Override public AuthenticationManagerBuilder eraseCredentials(boolean eraseCredentials) { authenticationBuilder.eraseCredentials(eraseCredentials); @@ -413,6 +416,9 @@ public abstract class WebSecurityConfigurerAdapter implements WebSecurityConfigu AuthenticationManagerDelegator(AuthenticationManagerBuilder delegateBuilder) { Assert.notNull(delegateBuilder,"delegateBuilder cannot be null"); + Field parentAuthMgrField = ReflectionUtils.findField(AuthenticationManagerBuilder.class, "parentAuthenticationManager"); + ReflectionUtils.makeAccessible(parentAuthMgrField); + validateBeanCycle(ReflectionUtils.getField(parentAuthMgrField, delegateBuilder)); this.delegateBuilder = delegateBuilder; } @@ -430,5 +436,18 @@ public abstract class WebSecurityConfigurerAdapter implements WebSecurityConfigu return delegate.authenticate(authentication); } + + private static void validateBeanCycle(Object auth) { + if(auth != null) { + String lazyBeanClassName = AuthenticationConfiguration.class.getName() + "$LazyBean"; + Class[] interfaces = auth.getClass().getInterfaces(); + for(Class i : interfaces) { + String className = i.getName(); + if(className.equals(lazyBeanClassName)) { + throw new FatalBeanException("A dependency cycle was detected when trying to resolve the AuthenticationManager. Please ensure you have configured authentication."); + } + } + } + } } } \ No newline at end of file diff --git a/config/src/test/groovy/org/springframework/security/config/annotation/web/configuration/Sec2515Tests.groovy b/config/src/test/groovy/org/springframework/security/config/annotation/web/configuration/Sec2515Tests.groovy new file mode 100644 index 0000000000..4bb75fa364 --- /dev/null +++ b/config/src/test/groovy/org/springframework/security/config/annotation/web/configuration/Sec2515Tests.groovy @@ -0,0 +1,72 @@ +/* + * 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.web.configuration; + +import org.springframework.beans.FatalBeanException; +import org.springframework.context.annotation.Bean +import org.springframework.context.annotation.Configuration +import org.springframework.security.authentication.AuthenticationManager +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken +import org.springframework.security.config.annotation.BaseSpringSpec +import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; + +public class Sec2515Tests extends BaseSpringSpec { + + def "SEC-2515: Prevent StackOverflow with bean graph cycle"() { + when: + loadConfig(StackOverflowSecurityConfig) + then: + thrown(FatalBeanException) + } + + @EnableWebSecurity + @Configuration + static class StackOverflowSecurityConfig extends WebSecurityConfigurerAdapter { + + @Override + @Bean + public AuthenticationManager authenticationManagerBean() + throws Exception { + return super.authenticationManagerBean(); + } + } + + + def "SEC-2515: @Bean still works when configure(AuthenticationManagerBuilder) used"() { + when: + loadConfig(SecurityConfig) + then: + noExceptionThrown(); + } + + @EnableWebSecurity + @Configuration + static class SecurityConfig extends WebSecurityConfigurerAdapter { + + @Override + @Bean + public AuthenticationManager authenticationManagerBean() + throws Exception { + return super.authenticationManagerBean(); + } + + @Override + protected void configure(AuthenticationManagerBuilder auth) + throws Exception { + auth.inMemoryAuthentication() + } + } +}