diff --git a/core/src/main/java/org/springframework/security/config/LdapConfigUtils.java b/core/src/main/java/org/springframework/security/config/LdapConfigUtils.java index 3435ffb92c..9acbe42423 100644 --- a/core/src/main/java/org/springframework/security/config/LdapConfigUtils.java +++ b/core/src/main/java/org/springframework/security/config/LdapConfigUtils.java @@ -1,13 +1,21 @@ package org.springframework.security.config; import org.springframework.security.ldap.SpringSecurityContextSource; +import org.springframework.security.providers.ldap.LdapAuthenticationProvider; +import org.springframework.security.providers.ldap.LdapAuthenticator; +import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanFactoryPostProcessor; +import org.springframework.beans.factory.config.BeanReference; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; +import org.springframework.beans.factory.config.RuntimeBeanReference; import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.beans.BeansException; import org.springframework.core.Ordered; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; import java.util.Map; /** @@ -17,35 +25,56 @@ import java.util.Map; */ class LdapConfigUtils { - /** Checks for the presence of a ContextSource instance */ + /** + * Checks for the presence of a ContextSource instance. Also supplies the standard reference to any + * unconfigured or beans. This is + * necessary in cases where the user has given the server a specific Id, but hasn't used + * the server-ref attribute to link this to the other ldap definitions. See SEC-799. + */ private static class ContextSourceSettingPostProcessor implements BeanFactoryPostProcessor, Ordered { + /** If set to true, a bean parser has indicated that the default context source name needs to be set */ + private boolean defaultNameRequired; + public void postProcessBeanFactory(ConfigurableListableBeanFactory bf) throws BeansException { - Map beans = bf.getBeansOfType(SpringSecurityContextSource.class); + String[] sources = bf.getBeanNamesForType(SpringSecurityContextSource.class); - if (beans.size() == 0) { + if (sources.length == 0) { throw new SecurityConfigurationException("No SpringSecurityContextSource instances found. Have you " + "added an <" + Elements.LDAP_SERVER + " /> element to your application context?"); } - -// else if (beans.size() > 1) { -// throw new SecurityConfigurationException("More than one SpringSecurityContextSource instance found. " + -// "Please specify a specific server id when configuring your <" + Elements.LDAP_PROVIDER + "> " + -// "or <" + Elements.LDAP_USER_SERVICE + ">."); -// } + + if (!bf.containsBean(BeanIds.CONTEXT_SOURCE) && defaultNameRequired) { + if (sources.length > 1) { + throw new SecurityConfigurationException("More than one SpringSecurityContextSource instance found. " + + "Please specify a specific server id using the 'server-ref' attribute when configuring your <" + + Elements.LDAP_PROVIDER + "> " + "or <" + Elements.LDAP_USER_SERVICE + ">."); + } + + bf.registerAlias(sources[0], BeanIds.CONTEXT_SOURCE); + } + } + + public void setDefaultNameRequired(boolean defaultNameRequired) { + this.defaultNameRequired = defaultNameRequired; } public int getOrder() { return LOWEST_PRECEDENCE; } } - - static void registerPostProcessorIfNecessary(BeanDefinitionRegistry registry) { + + static void registerPostProcessorIfNecessary(BeanDefinitionRegistry registry, boolean defaultNameRequired) { if (registry.containsBeanDefinition(BeanIds.CONTEXT_SOURCE_SETTING_POST_PROCESSOR)) { + if (defaultNameRequired) { + BeanDefinition bd = registry.getBeanDefinition(BeanIds.CONTEXT_SOURCE_SETTING_POST_PROCESSOR); + bd.getPropertyValues().addPropertyValue("defaultNameRequired", defaultNameRequired); + } return; } - registry.registerBeanDefinition(BeanIds.CONTEXT_SOURCE_SETTING_POST_PROCESSOR, - new RootBeanDefinition(ContextSourceSettingPostProcessor.class)); + BeanDefinition bd = new RootBeanDefinition(ContextSourceSettingPostProcessor.class); + registry.registerBeanDefinition(BeanIds.CONTEXT_SOURCE_SETTING_POST_PROCESSOR, bd); + bd.getPropertyValues().addPropertyValue("defaultNameRequired", defaultNameRequired); } } diff --git a/core/src/main/java/org/springframework/security/config/LdapProviderBeanDefinitionParser.java b/core/src/main/java/org/springframework/security/config/LdapProviderBeanDefinitionParser.java index 94ecf3d9b3..9cd00e1a66 100644 --- a/core/src/main/java/org/springframework/security/config/LdapProviderBeanDefinitionParser.java +++ b/core/src/main/java/org/springframework/security/config/LdapProviderBeanDefinitionParser.java @@ -94,8 +94,6 @@ public class LdapProviderBeanDefinitionParser implements BeanDefinitionParser { ldapProvider.getConstructorArgumentValues().addGenericArgumentValue(authenticator); ldapProvider.getConstructorArgumentValues().addGenericArgumentValue(LdapUserServiceBeanDefinitionParser.parseAuthoritiesPopulator(elt, parserContext)); - LdapConfigUtils.registerPostProcessorIfNecessary(parserContext.getRegistry()); - ConfigUtils.getRegisteredProviders(parserContext).add(ldapProvider); return null; diff --git a/core/src/main/java/org/springframework/security/config/LdapUserServiceBeanDefinitionParser.java b/core/src/main/java/org/springframework/security/config/LdapUserServiceBeanDefinitionParser.java index 32e842db9c..fe2f304e39 100644 --- a/core/src/main/java/org/springframework/security/config/LdapUserServiceBeanDefinitionParser.java +++ b/core/src/main/java/org/springframework/security/config/LdapUserServiceBeanDefinitionParser.java @@ -42,8 +42,6 @@ public class LdapUserServiceBeanDefinitionParser extends AbstractUserDetailsServ builder.addConstructorArg(parseSearchBean(elt, parserContext)); builder.addConstructorArg(parseAuthoritiesPopulator(elt, parserContext)); - - LdapConfigUtils.registerPostProcessorIfNecessary(parserContext.getRegistry()); } static RootBeanDefinition parseSearchBean(Element elt, ParserContext parserContext) { @@ -74,13 +72,16 @@ public class LdapUserServiceBeanDefinitionParser extends AbstractUserDetailsServ static RuntimeBeanReference parseServerReference(Element elt, ParserContext parserContext) { String server = elt.getAttribute(ATT_SERVER); - + boolean requiresDefaultName = false; + if (!StringUtils.hasText(server)) { server = BeanIds.CONTEXT_SOURCE; + requiresDefaultName = true; } RuntimeBeanReference contextSource = new RuntimeBeanReference(server); contextSource.setSource(parserContext.extractSource(elt)); + LdapConfigUtils.registerPostProcessorIfNecessary(parserContext.getRegistry(), requiresDefaultName); return contextSource; } diff --git a/core/src/test/java/org/springframework/security/config/LdapProviderBeanDefinitionParserTests.java b/core/src/test/java/org/springframework/security/config/LdapProviderBeanDefinitionParserTests.java index b1898eb6b3..43c2761f07 100644 --- a/core/src/test/java/org/springframework/security/config/LdapProviderBeanDefinitionParserTests.java +++ b/core/src/test/java/org/springframework/security/config/LdapProviderBeanDefinitionParserTests.java @@ -75,6 +75,12 @@ public class LdapProviderBeanDefinitionParserTests { LdapAuthenticationProvider provider = getProvider(); provider.authenticate(new UsernamePasswordAuthenticationToken("ben", "ben")); } + + @Test + public void detectsNonStandardServerId() { + setContext(" " + + ""); + } private void setContext(String context) { appCtx = new InMemoryXmlApplicationContext(context);