SEC-799: Add better detection of missing server-ref element for <ldap-user-service> and <ldap-authentication-provider />

http://jira.springframework.org/browse/SEC-799. Updated ContextSourceSettingPostProcessor to set the standard ContextSource as an alias if it is needed by a bean but has not been set (because the user specified their own server id on <ldap-server />).

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 <ldap-authentication-provider> or <ldap-user-service> 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);
     }
 
 }

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;

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;
     }

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("<ldap-server id='myServer'/> " +
+                "<ldap-authentication-provider />");
+    }
     
     private void setContext(String context) {
         appCtx = new InMemoryXmlApplicationContext(context);