From e40b9fbc75d189d8ca0b66ac6a1e1a8eaaadab98 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Mon, 3 Aug 2009 01:44:49 +0000 Subject: [PATCH] SEC-1196: Introduce AuthenticationManagerDelegator is MethodSecurityInterceptor which is configured by global-method-security. Prevents regression of SEC-933 caused by eager init of AuthenitcationManager and dependent beans --- .../security/config/BeanIds.java | 1 - ...balMethodSecurityBeanDefinitionParser.java | 37 ++++++++++++++++++- .../python-method-access-app-context.xml | 12 +++--- .../test/resources/sec-933-app-context.xml | 4 +- .../test/resources/sec-936-app-context.xml | 14 +++---- .../webapp/WEB-INF/in-memory-provider.xml | 16 ++++---- .../src/main/webapp/WEB-INF/ldap-provider.xml | 12 +++--- 7 files changed, 69 insertions(+), 27 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/BeanIds.java b/config/src/main/java/org/springframework/security/config/BeanIds.java index 12ad2cc679..29f7a2c4b1 100644 --- a/config/src/main/java/org/springframework/security/config/BeanIds.java +++ b/config/src/main/java/org/springframework/security/config/BeanIds.java @@ -27,7 +27,6 @@ public abstract class BeanIds { public static final String OPEN_ID_ENTRY_POINT = "_openIDFilterEntryPoint"; public static final String FILTER_CHAIN_PROXY = "_filterChainProxy"; - public static final String LDAP_AUTHENTICATION_PROVIDER = "_ldapAuthenticationProvider"; public static final String METHOD_SECURITY_METADATA_SOURCE_ADVISOR = "_methodSecurityMetadataSourceAdvisor"; public static final String EMBEDDED_APACHE_DS = "_apacheDirectoryServerContainer"; diff --git a/config/src/main/java/org/springframework/security/config/method/GlobalMethodSecurityBeanDefinitionParser.java b/config/src/main/java/org/springframework/security/config/method/GlobalMethodSecurityBeanDefinitionParser.java index 0486968ec0..6256292f9c 100644 --- a/config/src/main/java/org/springframework/security/config/method/GlobalMethodSecurityBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/method/GlobalMethodSecurityBeanDefinitionParser.java @@ -10,6 +10,9 @@ import java.util.Map; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.aop.config.AopNamespaceUtils; +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.BeanFactory; +import org.springframework.beans.factory.BeanFactoryAware; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.RuntimeBeanReference; import org.springframework.beans.factory.parsing.BeanComponentDefinition; @@ -38,7 +41,12 @@ import org.springframework.security.access.prepost.PrePostAnnotationSecurityMeta import org.springframework.security.access.vote.AffirmativeBased; import org.springframework.security.access.vote.AuthenticatedVoter; import org.springframework.security.access.vote.RoleVoter; +import org.springframework.security.authentication.AuthenticationManager; +import org.springframework.security.authentication.ProviderManager; import org.springframework.security.config.BeanIds; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.AuthenticationException; +import org.springframework.util.Assert; import org.springframework.util.StringUtils; import org.springframework.util.xml.DomUtils; import org.w3c.dom.Element; @@ -264,7 +272,7 @@ public class GlobalMethodSecurityBeanDefinitionParser implements BeanDefinitionP bldr.getRawBeanDefinition().setSource(source); bldr.addPropertyReference("accessDecisionManager", accessManagerId); - bldr.addPropertyReference("authenticationManager", BeanIds.AUTHENTICATION_MANAGER); + bldr.addPropertyValue("authenticationManager", new RootBeanDefinition(AuthenticationManagerDelegator.class)); bldr.addPropertyReference("securityMetadataSource", DELEGATING_METHOD_DEFINITION_SOURCE_ID); if (StringUtils.hasText(runAsManagerId)) { bldr.addPropertyReference("runAsManager", runAsManagerId); @@ -286,4 +294,31 @@ public class GlobalMethodSecurityBeanDefinitionParser implements BeanDefinitionP parserContext.getRegistry().registerBeanDefinition(BeanIds.METHOD_SECURITY_METADATA_SOURCE_ADVISOR, advisor); } + + /** + * Delays the lookup of the AuthenticationManager within MethodSecurityInterceptor, to prevent issues like SEC-933. + * + * @author Luke Taylor + * @since 3.0 + */ + public static final class AuthenticationManagerDelegator implements AuthenticationManager, BeanFactoryAware { + private AuthenticationManager delegate; + private final Object delegateMonitor = new Object(); + private BeanFactory beanFactory; + + public Authentication authenticate(Authentication authentication) throws AuthenticationException { + synchronized(delegateMonitor) { + if (delegate == null) { + Assert.state(beanFactory != null, "BeanFactory must be set to resolve " + BeanIds.AUTHENTICATION_MANAGER); + delegate = beanFactory.getBean(BeanIds.AUTHENTICATION_MANAGER, ProviderManager.class); + } + } + + return delegate.authenticate(authentication); + } + + public void setBeanFactory(BeanFactory beanFactory) throws BeansException { + this.beanFactory = beanFactory; + } + } } diff --git a/itest/context/src/test/resources/python-method-access-app-context.xml b/itest/context/src/test/resources/python-method-access-app-context.xml index 95266d00aa..5dd1778524 100755 --- a/itest/context/src/test/resources/python-method-access-app-context.xml +++ b/itest/context/src/test/resources/python-method-access-app-context.xml @@ -23,10 +23,12 @@ - - - - - + + + + + + + diff --git a/itest/context/src/test/resources/sec-933-app-context.xml b/itest/context/src/test/resources/sec-933-app-context.xml index 1f6da97afe..e89a9d6cae 100755 --- a/itest/context/src/test/resources/sec-933-app-context.xml +++ b/itest/context/src/test/resources/sec-933-app-context.xml @@ -12,8 +12,10 @@ - + + diff --git a/itest/context/src/test/resources/sec-936-app-context.xml b/itest/context/src/test/resources/sec-936-app-context.xml index e07fcbaa63..57ca6e7f71 100755 --- a/itest/context/src/test/resources/sec-936-app-context.xml +++ b/itest/context/src/test/resources/sec-936-app-context.xml @@ -7,13 +7,13 @@ http://www.springframework.org/schema/util http://www.springframework.org/schema/util/spring-util-2.5.xsd http://www.springframework.org/schema/security http://www.springframework.org/schema/security/spring-security.xsd"> - - - - - - - + + + + + + + diff --git a/itest/web/src/main/webapp/WEB-INF/in-memory-provider.xml b/itest/web/src/main/webapp/WEB-INF/in-memory-provider.xml index f4e5c98041..2e40e3ae05 100644 --- a/itest/web/src/main/webapp/WEB-INF/in-memory-provider.xml +++ b/itest/web/src/main/webapp/WEB-INF/in-memory-provider.xml @@ -6,12 +6,14 @@ xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-2.0.xsd http://www.springframework.org/schema/security http://www.springframework.org/schema/security/spring-security-3.0.xsd"> - - - - - - - + + + + + + + + + diff --git a/itest/web/src/main/webapp/WEB-INF/ldap-provider.xml b/itest/web/src/main/webapp/WEB-INF/ldap-provider.xml index 8aa82ad53a..9dd876f6a2 100644 --- a/itest/web/src/main/webapp/WEB-INF/ldap-provider.xml +++ b/itest/web/src/main/webapp/WEB-INF/ldap-provider.xml @@ -9,12 +9,14 @@ xmlns:beans="http://www.springframework.org/schema/beans" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-2.0.xsd - http://www.springframework.org/schema/security http://www.springframework.org/schema/security/spring-security-2.0.2.xsd"> + http://www.springframework.org/schema/security http://www.springframework.org/schema/security/spring-security.xsd"> - - - + + + + + - \ No newline at end of file +