From 670297c55df754d9185275764f9777bbabaf171a Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Thu, 14 Jan 2010 15:48:14 +0000 Subject: [PATCH] SEC-1369: Make sure beans aren't registered twice in case allowBeanDefinitionOverriding=false in the app context. The use of registerBeanComponent() also registers the bean definition, which causes an error if overriding is disallowed and the bean has already been registered using registerBeanDefinition(). I've also set the allowBeanDefinitionOverriding to 'false' on InMemoryXmlApplicationContext to detect future mistakes of this kind in testing. --- ...tractUserDetailsServiceBeanDefinitionParser.java | 4 +--- .../AuthenticationManagerBeanDefinitionParser.java | 9 ++++----- .../config/http/AuthenticationConfigBuilder.java | 13 ++++++------- ...ilterInvocationSecurityMetadataSourceParser.java | 2 +- .../config/http/HttpConfigurationBuilder.java | 12 ++++++------ .../http/HttpSecurityBeanDefinitionParser.java | 7 +++---- .../config/http/RememberMeBeanDefinitionParser.java | 2 +- .../GlobalMethodSecurityBeanDefinitionParser.java | 10 +++++----- .../config/util/InMemoryXmlApplicationContext.java | 1 + 9 files changed, 28 insertions(+), 32 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/authentication/AbstractUserDetailsServiceBeanDefinitionParser.java b/config/src/main/java/org/springframework/security/config/authentication/AbstractUserDetailsServiceBeanDefinitionParser.java index 1f34afc6ac..cf87fd8ec7 100644 --- a/config/src/main/java/org/springframework/security/config/authentication/AbstractUserDetailsServiceBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/authentication/AbstractUserDetailsServiceBeanDefinitionParser.java @@ -35,9 +35,8 @@ public abstract class AbstractUserDetailsServiceBeanDefinitionParser implements doParse(element, parserContext, builder); RootBeanDefinition userService = (RootBeanDefinition) builder.getBeanDefinition(); - String beanId = resolveId(element, userService, parserContext); + final String beanId = resolveId(element, userService, parserContext); - parserContext.getRegistry().registerBeanDefinition(beanId, userService); parserContext.registerBeanComponent(new BeanComponentDefinition(userService, beanId)); String cacheRef = element.getAttribute(CACHE_REF); @@ -49,7 +48,6 @@ public abstract class AbstractUserDetailsServiceBeanDefinitionParser implements cachingUSBuilder.addPropertyValue("userCache", new RuntimeBeanReference(cacheRef)); BeanDefinition cachingUserService = cachingUSBuilder.getBeanDefinition(); - parserContext.getRegistry().registerBeanDefinition(beanId + CACHING_SUFFIX, cachingUserService); parserContext.registerBeanComponent(new BeanComponentDefinition(cachingUserService, beanId + CACHING_SUFFIX)); } diff --git a/config/src/main/java/org/springframework/security/config/authentication/AuthenticationManagerBeanDefinitionParser.java b/config/src/main/java/org/springframework/security/config/authentication/AuthenticationManagerBeanDefinitionParser.java index b517967a5e..90ed110cb4 100644 --- a/config/src/main/java/org/springframework/security/config/authentication/AuthenticationManagerBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/authentication/AuthenticationManagerBeanDefinitionParser.java @@ -60,7 +60,7 @@ public class AuthenticationManagerBeanDefinitionParser implements BeanDefinition } else { BeanDefinition provider = resolver.resolve(providerElt.getNamespaceURI()).parse(providerElt, pc); Assert.notNull(provider, "Parser for " + providerElt.getNodeName() + " returned a null bean definition"); - String id = pc.getReaderContext().registerWithGeneratedName(provider); + String id = pc.getReaderContext().generateBeanName(provider); pc.registerBeanComponent(new BeanComponentDefinition(provider, id)); providers.add(new RuntimeBeanReference(id)); } @@ -74,13 +74,12 @@ public class AuthenticationManagerBeanDefinitionParser implements BeanDefinition providerManagerBldr.addPropertyValue("providers", providers); // Add the default event publisher BeanDefinition publisher = new RootBeanDefinition(DefaultAuthenticationEventPublisher.class); - String id = pc.getReaderContext().registerWithGeneratedName(publisher); + String id = pc.getReaderContext().generateBeanName(publisher); pc.registerBeanComponent(new BeanComponentDefinition(publisher, id)); providerManagerBldr.addPropertyReference("authenticationEventPublisher", id); - BeanDefinition authManager = providerManagerBldr.getBeanDefinition(); - pc.getRegistry().registerBeanDefinition(BeanIds.AUTHENTICATION_MANAGER, authManager); - pc.registerBeanComponent(new BeanComponentDefinition(authManager, BeanIds.AUTHENTICATION_MANAGER)); + pc.registerBeanComponent( + new BeanComponentDefinition(providerManagerBldr.getBeanDefinition(), BeanIds.AUTHENTICATION_MANAGER)); if (StringUtils.hasText(alias)) { pc.getRegistry().registerAlias(BeanIds.AUTHENTICATION_MANAGER, alias); diff --git a/config/src/main/java/org/springframework/security/config/http/AuthenticationConfigBuilder.java b/config/src/main/java/org/springframework/security/config/http/AuthenticationConfigBuilder.java index 448c025c2d..866d47aed2 100644 --- a/config/src/main/java/org/springframework/security/config/http/AuthenticationConfigBuilder.java +++ b/config/src/main/java/org/springframework/security/config/http/AuthenticationConfigBuilder.java @@ -143,7 +143,7 @@ final class AuthenticationConfigBuilder { provider.getPropertyValues().addPropertyValue("key", key); - String id = pc.getReaderContext().registerWithGeneratedName(provider); + String id = pc.getReaderContext().generateBeanName(provider); pc.registerBeanComponent(new BeanComponentDefinition(provider, id)); rememberMeProviderRef = new RuntimeBeanReference(id); @@ -168,7 +168,7 @@ final class AuthenticationConfigBuilder { // Id is required by login page filter - formFilterId = pc.getReaderContext().registerWithGeneratedName(formFilter); + formFilterId = pc.getReaderContext().generateBeanName(formFilter); pc.registerBeanComponent(new BeanComponentDefinition(formFilter, formFilterId)); injectRememberMeServicesRef(formFilter, rememberMeServicesId); } @@ -217,8 +217,7 @@ final class AuthenticationConfigBuilder { openIDFilter.getPropertyValues().addPropertyValue("allowSessionCreation", new Boolean(allowSessionCreation)); openIDFilter.getPropertyValues().addPropertyValue("authenticationManager", authManager); // Required by login page filter - openIDFilterId = pc.getReaderContext().registerWithGeneratedName(openIDFilter); - pc.getRegistry().registerBeanDefinition(openIDFilterId, openIDFilter); + openIDFilterId = pc.getReaderContext().generateBeanName(openIDFilter); pc.registerBeanComponent(new BeanComponentDefinition(openIDFilter, openIDFilterId)); injectRememberMeServicesRef(openIDFilter, rememberMeServicesId); @@ -266,7 +265,7 @@ final class AuthenticationConfigBuilder { entryPoint.getPropertyValues().addPropertyValue("realmName", realm); - String entryPointId = pc.getReaderContext().registerWithGeneratedName(entryPoint); + String entryPointId = pc.getReaderContext().generateBeanName(entryPoint); pc.registerBeanComponent(new BeanComponentDefinition(entryPoint, entryPointId)); filterBuilder.addPropertyValue("authenticationManager", authManager); @@ -398,7 +397,7 @@ final class AuthenticationConfigBuilder { RootBeanDefinition anonymousProviderBean = new RootBeanDefinition(AnonymousAuthenticationProvider.class); anonymousProviderBean.setSource(anonymousFilter.getSource()); anonymousProviderBean.getPropertyValues().addPropertyValue(keyPV); - String id = pc.getReaderContext().registerWithGeneratedName(anonymousProviderBean); + String id = pc.getReaderContext().generateBeanName(anonymousProviderBean); pc.registerBeanComponent(new BeanComponentDefinition(anonymousProviderBean, id)); anonymousProviderRef = new RuntimeBeanReference(id); @@ -430,7 +429,7 @@ final class AuthenticationConfigBuilder { requestCacheBldr.addPropertyValue("portResolver", portResolver.getBeanDefinition()); BeanDefinition bean = requestCacheBldr.getBeanDefinition(); - String id = pc.getReaderContext().registerWithGeneratedName(bean); + String id = pc.getReaderContext().generateBeanName(bean); pc.registerBeanComponent(new BeanComponentDefinition(bean, id)); this.requestCache = new RuntimeBeanReference(id); diff --git a/config/src/main/java/org/springframework/security/config/http/FilterInvocationSecurityMetadataSourceParser.java b/config/src/main/java/org/springframework/security/config/http/FilterInvocationSecurityMetadataSourceParser.java index 66cdf3dcc9..6505a7701a 100644 --- a/config/src/main/java/org/springframework/security/config/http/FilterInvocationSecurityMetadataSourceParser.java +++ b/config/src/main/java/org/springframework/security/config/http/FilterInvocationSecurityMetadataSourceParser.java @@ -78,7 +78,7 @@ public class FilterInvocationSecurityMetadataSourceParser implements BeanDefinit logger.info("Using bean '" + expressionHandlerRef + "' as web SecurityExpressionHandler implementation"); } else { BeanDefinition expressionHandler = BeanDefinitionBuilder.rootBeanDefinition(DefaultWebSecurityExpressionHandler.class).getBeanDefinition(); - expressionHandlerRef = pc.getReaderContext().registerWithGeneratedName(expressionHandler); + expressionHandlerRef = pc.getReaderContext().generateBeanName(expressionHandler); pc.registerBeanComponent(new BeanComponentDefinition(expressionHandler, expressionHandlerRef)); } diff --git a/config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java b/config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java index 25eb38ad79..ef0df1e8f7 100644 --- a/config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java +++ b/config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java @@ -175,7 +175,7 @@ class HttpConfigurationBuilder { } BeanDefinition repoBean = contextRepo.getBeanDefinition(); - repoRef = pc.getReaderContext().registerWithGeneratedName(repoBean); + repoRef = pc.getReaderContext().generateBeanName(repoBean); pc.registerBeanComponent(new BeanComponentDefinition(repoBean, repoRef)); } @@ -261,7 +261,7 @@ class HttpConfigurationBuilder { sessionStrategy.addPropertyValue("migrateSessionAttributes", Boolean.valueOf(sessionFixationAttribute.equals(OPT_SESSION_FIXATION_MIGRATE_SESSION))); } - sessionAuthStratRef = pc.getReaderContext().registerWithGeneratedName(strategyBean); + sessionAuthStratRef = pc.getReaderContext().generateBeanName(strategyBean); pc.registerBeanComponent(new BeanComponentDefinition(strategyBean, sessionAuthStratRef)); } @@ -427,7 +427,7 @@ class HttpConfigurationBuilder { String accessManagerId = httpElt.getAttribute(ATT_ACCESS_MGR); if (!StringUtils.hasText(accessManagerId)) { - accessManagerId = pc.getReaderContext().registerWithGeneratedName(accessDecisionMgr); + accessManagerId = pc.getReaderContext().generateBeanName(accessDecisionMgr); pc.registerBeanComponent(new BeanComponentDefinition(accessDecisionMgr, accessManagerId)); } @@ -442,14 +442,14 @@ class HttpConfigurationBuilder { builder.addPropertyValue("securityMetadataSource", securityMds); BeanDefinition fsiBean = builder.getBeanDefinition(); - String fsiId = pc.getReaderContext().registerWithGeneratedName(fsiBean); + String fsiId = pc.getReaderContext().generateBeanName(fsiBean); pc.registerBeanComponent(new BeanComponentDefinition(fsiBean,fsiId)); // Create and register a DefaultWebInvocationPrivilegeEvaluator for use with taglibs etc. BeanDefinition wipe = new RootBeanDefinition(DefaultWebInvocationPrivilegeEvaluator.class); wipe.getConstructorArgumentValues().addGenericArgumentValue(new RuntimeBeanReference(fsiId)); - String wipeId = pc.getReaderContext().registerWithGeneratedName(wipe); - pc.registerBeanComponent(new BeanComponentDefinition(wipe, wipeId)); + + pc.registerBeanComponent(new BeanComponentDefinition(wipe, pc.getReaderContext().generateBeanName(wipe))); this.fsi = new RuntimeBeanReference(fsiId); } diff --git a/config/src/main/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParser.java b/config/src/main/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParser.java index b6f89ffd09..b1a5588322 100644 --- a/config/src/main/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParser.java @@ -155,7 +155,7 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser { // Register the portMapper. A default will always be created, even if no element exists. BeanDefinition portMapper = new PortMappingsBeanDefinitionParser().parse( DomUtils.getChildElementByTagName(elt, Elements.PORT_MAPPINGS), pc); - String portMapperName = pc.getReaderContext().registerWithGeneratedName(portMapper); + String portMapperName = pc.getReaderContext().generateBeanName(portMapper); pc.registerBeanComponent(new BeanComponentDefinition(portMapper, portMapperName)); return portMapperName; @@ -179,7 +179,7 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser { } authManager.getRawBeanDefinition().setSource(pc.extractSource(element)); BeanDefinition authMgrBean = authManager.getBeanDefinition(); - String id = pc.getReaderContext().registerWithGeneratedName(authMgrBean); + String id = pc.getReaderContext().generateBeanName(authMgrBean); pc.registerBeanComponent(new BeanComponentDefinition(authMgrBean, id)); return new RuntimeBeanReference(id); @@ -263,9 +263,8 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser { fcpBldr.addPropertyValue("stripQueryStringFromUrls", Boolean.valueOf(matcher instanceof AntUrlPathMatcher)); fcpBldr.addPropertyValue("filterChainMap", filterChainMap); BeanDefinition fcpBean = fcpBldr.getBeanDefinition(); - pc.getRegistry().registerBeanDefinition(BeanIds.FILTER_CHAIN_PROXY, fcpBean); - pc.getRegistry().registerAlias(BeanIds.FILTER_CHAIN_PROXY, BeanIds.SPRING_SECURITY_FILTER_CHAIN); pc.registerBeanComponent(new BeanComponentDefinition(fcpBean, BeanIds.FILTER_CHAIN_PROXY)); + pc.getRegistry().registerAlias(BeanIds.FILTER_CHAIN_PROXY, BeanIds.SPRING_SECURITY_FILTER_CHAIN); } static UrlMatcher createUrlMatcher(Element element) { diff --git a/config/src/main/java/org/springframework/security/config/http/RememberMeBeanDefinitionParser.java b/config/src/main/java/org/springframework/security/config/http/RememberMeBeanDefinitionParser.java index c6dbf713db..c4c259822c 100644 --- a/config/src/main/java/org/springframework/security/config/http/RememberMeBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/http/RememberMeBeanDefinitionParser.java @@ -105,7 +105,7 @@ class RememberMeBeanDefinitionParser implements BeanDefinitionParser { } services.setSource(source); services.getPropertyValues().addPropertyValue("key", key); - servicesName = pc.getReaderContext().registerWithGeneratedName(services); + servicesName = pc.getReaderContext().generateBeanName(services); pc.registerBeanComponent(new BeanComponentDefinition(services, servicesName)); } else { servicesName = rememberMeServicesRef; 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 618b264979..774155ff73 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 @@ -128,7 +128,7 @@ public class GlobalMethodSecurityBeanDefinitionParser implements BeanDefinitionP logger.info("Using bean '" + expressionHandlerRef + "' as method ExpressionHandler implementation"); } else { BeanDefinition expressionHandler = new RootBeanDefinition(DefaultMethodSecurityExpressionHandler.class); - expressionHandlerRef = pc.getReaderContext().registerWithGeneratedName(expressionHandler); + expressionHandlerRef = pc.getReaderContext().generateBeanName(expressionHandler); pc.registerBeanComponent(new BeanComponentDefinition(expressionHandler, expressionHandlerRef)); logger.info("Expressions were enabled for method security but no SecurityExpressionHandler was configured. " + "All hasPermision() expressions will evaluate to false."); @@ -167,7 +167,7 @@ public class GlobalMethodSecurityBeanDefinitionParser implements BeanDefinitionP if (pointcutMap.size() > 0) { // Only add it if there are actually any pointcuts defined. BeanDefinition mapBasedMetadataSource = new RootBeanDefinition(MapBasedMethodSecurityMetadataSource.class); - BeanReference ref = new RuntimeBeanReference(pc.getReaderContext().registerWithGeneratedName(mapBasedMetadataSource)); + BeanReference ref = new RuntimeBeanReference(pc.getReaderContext().generateBeanName(mapBasedMetadataSource)); delegates.add(ref); pc.registerBeanComponent(new BeanComponentDefinition(mapBasedMetadataSource, ref.getBeanName())); @@ -226,7 +226,7 @@ public class GlobalMethodSecurityBeanDefinitionParser implements BeanDefinitionP accessMgrBuilder.addPropertyValue("decisionVoters", voters); BeanDefinition accessManager = accessMgrBuilder.getBeanDefinition(); - String id = pc.getReaderContext().registerWithGeneratedName(accessManager); + String id = pc.getReaderContext().generateBeanName(accessManager); pc.registerBeanComponent(new BeanComponentDefinition(accessManager, id)); return id; @@ -238,7 +238,7 @@ public class GlobalMethodSecurityBeanDefinitionParser implements BeanDefinitionP delegatingMethodSecurityMetadataSource.setSource(source); delegatingMethodSecurityMetadataSource.getPropertyValues().addPropertyValue("methodSecurityMetadataSources", delegates); - String id = pc.getReaderContext().registerWithGeneratedName(delegatingMethodSecurityMetadataSource); + String id = pc.getReaderContext().generateBeanName(delegatingMethodSecurityMetadataSource); pc.registerBeanComponent(new BeanComponentDefinition(delegatingMethodSecurityMetadataSource, id)); return new RuntimeBeanReference(id); @@ -302,7 +302,7 @@ public class GlobalMethodSecurityBeanDefinitionParser implements BeanDefinitionP } BeanDefinition bean = bldr.getBeanDefinition(); - String id = pc.getReaderContext().registerWithGeneratedName(bean); + String id = pc.getReaderContext().generateBeanName(bean); pc.registerBeanComponent(new BeanComponentDefinition(bean, id)); return new RuntimeBeanReference(id); diff --git a/config/src/test/java/org/springframework/security/config/util/InMemoryXmlApplicationContext.java b/config/src/test/java/org/springframework/security/config/util/InMemoryXmlApplicationContext.java index 8d4c1259a5..7b5beef2ce 100644 --- a/config/src/test/java/org/springframework/security/config/util/InMemoryXmlApplicationContext.java +++ b/config/src/test/java/org/springframework/security/config/util/InMemoryXmlApplicationContext.java @@ -32,6 +32,7 @@ public class InMemoryXmlApplicationContext extends AbstractXmlApplicationContext public InMemoryXmlApplicationContext(String xml, String secVersion, ApplicationContext parent) { String fullXml = BEANS_OPENING + secVersion + ".xsd'>\n" + xml + BEANS_CLOSE; inMemoryXml = new InMemoryResource(fullXml); + setAllowBeanDefinitionOverriding(false); setParent(parent); refresh(); }