From 5e88ebef2e973d4734cc30a392d1a7ec15e97b17 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Thu, 24 Apr 2014 13:55:25 -0500 Subject: [PATCH] SEC-2549: Remove LazyBean marker interface --- .../AuthenticationConfiguration.java | 3 -- .../WebSecurityConfigurerAdapter.java | 39 +++++++++++----- .../config/annotation/BaseSpringSpec.groovy | 3 +- .../web/configuration/Sec2515Tests.groovy | 45 +++++++++++++++++++ 4 files changed, 75 insertions(+), 15 deletions(-) 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 2029d45099..a007e6c63e 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 @@ -115,7 +115,6 @@ public class AuthenticationConfiguration { ProxyFactoryBean proxyFactory = new ProxyFactoryBean(); proxyFactory = objectPostProcessor.postProcess(proxyFactory); proxyFactory.setTargetSource(lazyTargetSource); - proxyFactory.setInterfaces(new Class[] { interfaceName, LazyBean.class }); return (T) proxyFactory.getObject(); } @@ -123,8 +122,6 @@ public class AuthenticationConfiguration { return lazyBean(AuthenticationManager.class); } - private interface LazyBean {} - private static class EnableGlobalAuthenticationAutowiredConfigurer extends GlobalAuthenticationConfigurerAdapter { private final ApplicationContext context; private static final Log logger = LogFactory.getLog(EnableGlobalAuthenticationAutowiredConfigurer.class); 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 79f3ee822b..7404c5ecc6 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 @@ -18,11 +18,18 @@ package org.springframework.security.config.annotation.web.configuration; import java.lang.reflect.Field; import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.aop.TargetSource; +import org.springframework.aop.framework.Advised; +import org.springframework.aop.target.LazyInitTargetSource; import org.springframework.beans.FatalBeanException; +import org.springframework.beans.factory.BeanFactoryUtils; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationContext; import org.springframework.core.annotation.Order; @@ -210,7 +217,7 @@ public abstract class WebSecurityConfigurerAdapter implements WebSecurityConfigu * @throws Exception */ public AuthenticationManager authenticationManagerBean() throws Exception { - return new AuthenticationManagerDelegator(authenticationBuilder); + return new AuthenticationManagerDelegator(authenticationBuilder, context); } /** @@ -413,12 +420,14 @@ public abstract class WebSecurityConfigurerAdapter implements WebSecurityConfigu private AuthenticationManagerBuilder delegateBuilder; private AuthenticationManager delegate; private final Object delegateMonitor = new Object(); + private Set beanNames; - AuthenticationManagerDelegator(AuthenticationManagerBuilder delegateBuilder) { + AuthenticationManagerDelegator(AuthenticationManagerBuilder delegateBuilder, ApplicationContext context) { Assert.notNull(delegateBuilder,"delegateBuilder cannot be null"); Field parentAuthMgrField = ReflectionUtils.findField(AuthenticationManagerBuilder.class, "parentAuthenticationManager"); ReflectionUtils.makeAccessible(parentAuthMgrField); - validateBeanCycle(ReflectionUtils.getField(parentAuthMgrField, delegateBuilder)); + beanNames = getAuthenticationManagerBeanNames(context); + validateBeanCycle(ReflectionUtils.getField(parentAuthMgrField, delegateBuilder), beanNames); this.delegateBuilder = delegateBuilder; } @@ -437,16 +446,24 @@ 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."); + private static Set getAuthenticationManagerBeanNames(ApplicationContext applicationContext) { + String[] beanNamesForType = BeanFactoryUtils.beanNamesForTypeIncludingAncestors(applicationContext, AuthenticationManager.class); + return new HashSet(Arrays.asList(beanNamesForType)); + } + + private static void validateBeanCycle(Object auth, Set beanNames) { + if(auth != null && !beanNames.isEmpty()) { + if(auth instanceof Advised){ + Advised advised = (Advised) auth; + TargetSource targetSource = advised.getTargetSource(); + if(targetSource instanceof LazyInitTargetSource) { + LazyInitTargetSource lits = (LazyInitTargetSource) targetSource; + if(beanNames.contains(lits.getTargetBeanName())) { + throw new FatalBeanException("A dependency cycle was detected when trying to resolve the AuthenticationManager. Please ensure you have configured authentication."); + } } } + beanNames = Collections.emptySet(); } } } 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 0a3068f269..c6942fae14 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 @@ -123,7 +123,8 @@ abstract class BaseSpringSpec extends Specification { AuthenticationManager getAuthenticationManager() { try { authenticationManager().delegateBuilder.getObject() - } catch(NoSuchBeanDefinitionException e) {} + } catch(NoSuchBeanDefinitionException e) { + } catch(MissingPropertyException e) {} findFilter(FilterSecurityInterceptor).authenticationManager } 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 index 4bb75fa364..574af70102 100644 --- 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 @@ -15,10 +15,13 @@ */ package org.springframework.security.config.annotation.web.configuration; +import org.springframework.beans.factory.annotation.Autowired import org.springframework.beans.FatalBeanException; +import org.springframework.context.annotation.AnnotationConfigApplicationContext import org.springframework.context.annotation.Bean import org.springframework.context.annotation.Configuration import org.springframework.security.authentication.AuthenticationManager +import org.springframework.security.authentication.TestingAuthenticationToken import org.springframework.security.authentication.UsernamePasswordAuthenticationToken import org.springframework.security.config.annotation.BaseSpringSpec import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; @@ -44,6 +47,48 @@ public class Sec2515Tests extends BaseSpringSpec { } } + def "Custom Name Prevent StackOverflow with bean graph cycle"() { + when: + loadConfig(StackOverflowSecurityConfig) + then: + thrown(FatalBeanException) + } + + @EnableWebSecurity + @Configuration + static class CustomBeanNameStackOverflowSecurityConfig extends WebSecurityConfigurerAdapter { + + @Override + @Bean(name="custom") + public AuthenticationManager authenticationManagerBean() + throws Exception { + return super.authenticationManagerBean(); + } + } + + def "SEC-2549: Can load with child classloader"() { + setup: + CanLoadWithChildConfig.AM = Mock(AuthenticationManager) + context = new AnnotationConfigApplicationContext() + context.classLoader = new URLClassLoader(new URL[0], context.classLoader) + context.register(CanLoadWithChildConfig) + context.refresh() + when: + authenticationManager.authenticate(new UsernamePasswordAuthenticationToken("user", "password")) + then: + noExceptionThrown() + 1 * CanLoadWithChildConfig.AM.authenticate(_) >> new TestingAuthenticationToken("user","password","ROLE_USER") + } + + @EnableWebSecurity + @Configuration + static class CanLoadWithChildConfig extends WebSecurityConfigurerAdapter { + static AuthenticationManager AM + @Bean + public AuthenticationManager am() { + AM + } + } def "SEC-2515: @Bean still works when configure(AuthenticationManagerBuilder) used"() { when: