Refactoring to remove remaining circular dependencies indicated by structure101.

config/src/main/java/org/springframework/security/config/authentication/AbstractUserDetailsServiceBeanDefinitionParser.java

@@ -1,18 +1,16 @@
 package org.springframework.security.config.authentication;
 
-import org.springframework.beans.factory.xml.BeanDefinitionParser;
-import org.springframework.beans.factory.xml.ParserContext;
+import org.springframework.beans.factory.BeanDefinitionStoreException;
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.config.RuntimeBeanReference;
 import org.springframework.beans.factory.parsing.BeanComponentDefinition;
 import org.springframework.beans.factory.support.AbstractBeanDefinition;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
 import org.springframework.beans.factory.support.RootBeanDefinition;
-import org.springframework.beans.factory.BeanDefinitionStoreException;
+import org.springframework.beans.factory.xml.BeanDefinitionParser;
+import org.springframework.beans.factory.xml.ParserContext;
 import org.springframework.security.config.BeanIds;
-import org.springframework.security.config.Elements;
 import org.springframework.util.StringUtils;
-
 import org.w3c.dom.Element;
 
 /**
@@ -22,9 +20,6 @@ public abstract class AbstractUserDetailsServiceBeanDefinitionParser implements
     static final String CACHE_REF = "cache-ref";
     public static final String CACHING_SUFFIX = ".caching";
 
-    /**  UserDetailsService bean Id. For use in a stateful context (i.e. in AuthenticationProviderBDP) */
-    private String id;
-
     protected abstract String getBeanClassName(Element element);
 
     protected abstract void doParse(Element element, ParserContext parserContext, BeanDefinitionBuilder builder);
@@ -51,34 +46,33 @@ public abstract class AbstractUserDetailsServiceBeanDefinitionParser implements
             parserContext.registerBeanComponent(new BeanComponentDefinition(cachingUserService, beanId + CACHING_SUFFIX));
         }
 
-        id = beanId;
-
         return null;
     }
 
-    private String resolveId(Element element, AbstractBeanDefinition definition, ParserContext parserContext)
+    private String resolveId(Element element, AbstractBeanDefinition definition, ParserContext pc)
             throws BeanDefinitionStoreException {
 
         String id = element.getAttribute("id");
 
-        if (StringUtils.hasText(id)) {
-            return id;
+        if (pc.isNested()) {
+            // We're inside an <authentication-provider> element
+            if (!StringUtils.hasText(id)) {
+                id = pc.getReaderContext().generateBeanName(definition);
+            }
+            BeanDefinition container = pc.getContainingBeanDefinition();
+            container.getPropertyValues().add("userDetailsService", new RuntimeBeanReference(id));
         }
 
-        if(Elements.AUTHENTICATION_PROVIDER.equals(element.getParentNode().getNodeName())) {
-            return parserContext.getReaderContext().generateBeanName(definition);
+        if (StringUtils.hasText(id)) {
+            return id;
         }
 
         // If top level, use the default name or throw an exception if already used
-        if (parserContext.getRegistry().containsBeanDefinition(BeanIds.USER_DETAILS_SERVICE)) {
+        if (pc.getRegistry().containsBeanDefinition(BeanIds.USER_DETAILS_SERVICE)) {
             throw new BeanDefinitionStoreException("No id supplied and another " +
                     "bean is already registered as " + BeanIds.USER_DETAILS_SERVICE);
         }
 
         return BeanIds.USER_DETAILS_SERVICE;
     }
-
-    String getId() {
-        return id;
-    }
 }

config/src/main/java/org/springframework/security/config/authentication/AuthenticationProviderBeanDefinitionParser.java

@@ -7,7 +7,6 @@ import org.springframework.beans.factory.xml.BeanDefinitionParser;
 import org.springframework.beans.factory.xml.ParserContext;
 import org.springframework.security.authentication.dao.DaoAuthenticationProvider;
 import org.springframework.security.config.Elements;
-import org.springframework.security.config.ldap.LdapUserServiceBeanDefinitionParser;
 import org.springframework.util.StringUtils;
 import org.springframework.util.xml.DomUtils;
 import org.w3c.dom.Element;
@@ -21,14 +20,14 @@ import org.w3c.dom.Element;
 public class AuthenticationProviderBeanDefinitionParser implements BeanDefinitionParser {
     private static String ATT_USER_DETAILS_REF = "user-service-ref";
 
-    public BeanDefinition parse(Element element, ParserContext parserContext) {
+    public BeanDefinition parse(Element element, ParserContext pc) {
         RootBeanDefinition authProvider = new RootBeanDefinition(DaoAuthenticationProvider.class);
-        authProvider.setSource(parserContext.extractSource(element));
+        authProvider.setSource(pc.extractSource(element));
 
         Element passwordEncoderElt = DomUtils.getChildElementByTagName(element, Elements.PASSWORD_ENCODER);
 
         if (passwordEncoderElt != null) {
-            PasswordEncoderParser pep = new PasswordEncoderParser(passwordEncoderElt, parserContext);
+            PasswordEncoderParser pep = new PasswordEncoderParser(passwordEncoderElt, pc);
             authProvider.getPropertyValues().addPropertyValue("passwordEncoder", pep.getPasswordEncoder());
 
             if (pep.getSaltSource() != null) {
@@ -37,98 +36,39 @@ public class AuthenticationProviderBeanDefinitionParser implements BeanDefinitio
         }
 
         Element userServiceElt = DomUtils.getChildElementByTagName(element, Elements.USER_SERVICE);
-        Element jdbcUserServiceElt = DomUtils.getChildElementByTagName(element, Elements.JDBC_USER_SERVICE);
-        Element ldapUserServiceElt = DomUtils.getChildElementByTagName(element, Elements.LDAP_USER_SERVICE);
+        if (userServiceElt == null) {
+            userServiceElt = DomUtils.getChildElementByTagName(element, Elements.JDBC_USER_SERVICE);
+        }
+        if (userServiceElt == null) {
+            userServiceElt = DomUtils.getChildElementByTagName(element, Elements.LDAP_USER_SERVICE);
+        }
 
         String ref = element.getAttribute(ATT_USER_DETAILS_REF);
 
         if (StringUtils.hasText(ref)) {
-            if (userServiceElt != null || jdbcUserServiceElt != null || ldapUserServiceElt != null) {
-                parserContext.getReaderContext().error("The " + ATT_USER_DETAILS_REF + " attribute cannot be used in combination with child" +
+            if (userServiceElt != null) {
+                pc.getReaderContext().error("The " + ATT_USER_DETAILS_REF + " attribute cannot be used in combination with child" +
                         "elements '" + Elements.USER_SERVICE + "', '" + Elements.JDBC_USER_SERVICE + "' or '" +
                         Elements.LDAP_USER_SERVICE + "'", element);
             }
+
+            authProvider.getPropertyValues().add("userDetailsService", new RuntimeBeanReference(ref));
         } else {
             // Use the child elements to create the UserDetailsService
-            AbstractUserDetailsServiceBeanDefinitionParser parser = null;
-            Element elt = null;
-
             if (userServiceElt != null) {
-                elt = userServiceElt;
-                parser = new UserServiceBeanDefinitionParser();
-            } else if (jdbcUserServiceElt != null) {
-                elt = jdbcUserServiceElt;
-                parser = new JdbcUserServiceBeanDefinitionParser();
-            } else if (ldapUserServiceElt != null) {
-                elt = ldapUserServiceElt;
-                parser = new LdapUserServiceBeanDefinitionParser();
+                pc.getDelegate().parseCustomElement(userServiceElt, authProvider);
             } else {
-                parserContext.getReaderContext().error("A user-service is required", element);
+                pc.getReaderContext().error("A user-service is required", element);
             }
 
-            parser.parse(elt, parserContext);
-            ref = parser.getId();
-
             // Pinch the cache-ref from the UserDetailService element, if set.
-            String cacheRef = elt.getAttribute(AbstractUserDetailsServiceBeanDefinitionParser.CACHE_REF);
+            String cacheRef = userServiceElt.getAttribute(AbstractUserDetailsServiceBeanDefinitionParser.CACHE_REF);
 
             if (StringUtils.hasText(cacheRef)) {
                 authProvider.getPropertyValues().addPropertyValue("userCache", new RuntimeBeanReference(cacheRef));
             }
         }
 
-        authProvider.getPropertyValues().addPropertyValue("userDetailsService", new RuntimeBeanReference(ref));
-
-        // We need to register the provider to access it in the post processor to check if it has a cache
-//        final String id = parserContext.getReaderContext().generateBeanName(authProvider);
-//        parserContext.getRegistry().registerBeanDefinition(id, authProvider);
-//        parserContext.registerComponent(new BeanComponentDefinition(authProvider, id));
-
-
-//        BeanDefinitionBuilder cacheResolverBldr = BeanDefinitionBuilder.rootBeanDefinition(AuthenticationProviderCacheResolver.class);
-//        cacheResolverBldr.addConstructorArgValue(id);
-//        cacheResolverBldr.addConstructorArgValue(ref);
-//        cacheResolverBldr.setRole(BeanDefinition.ROLE_INFRASTRUCTURE);
-//        BeanDefinition cacheResolver = cacheResolverBldr.getBeanDefinition();
-//
-//        String name = parserContext.getReaderContext().generateBeanName(cacheResolver);
-//        parserContext.getRegistry().registerBeanDefinition(name , cacheResolver);
-//        parserContext.registerComponent(new BeanComponentDefinition(cacheResolver, name));
-
-//        ConfigUtils.addAuthenticationProvider(parserContext, id, element);
-
         return authProvider;
     }
-
-    /**
-     * Checks whether the registered user service bean has an associated cache and, if so, sets it on the
-     * authentication provider.
-     */
-//    static class AuthenticationProviderCacheResolver implements BeanFactoryPostProcessor, Ordered {
-//        private String providerId;
-//        private String userServiceId;
-//
-//        public AuthenticationProviderCacheResolver(String providerId, String userServiceId) {
-//            this.providerId = providerId;
-//            this.userServiceId = userServiceId;
-//        }
-//
-//        public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) throws BeansException {
-//            RootBeanDefinition provider = (RootBeanDefinition) beanFactory.getBeanDefinition(providerId);
-//
-//            String cachingId = userServiceId + AbstractUserDetailsServiceBeanDefinitionParser.CACHING_SUFFIX;
-//
-//            if (beanFactory.containsBeanDefinition(cachingId)) {
-//                RootBeanDefinition cachingUserService = (RootBeanDefinition) beanFactory.getBeanDefinition(cachingId);
-//
-//                PropertyValue userCacheProperty = cachingUserService.getPropertyValues().getPropertyValue("userCache");
-//
-//                provider.getPropertyValues().addPropertyValue(userCacheProperty);
-//            }
-//        }
-//
-//        public int getOrder() {
-//            return HIGHEST_PRECEDENCE;
-//        }
-//    }
 }

ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsService.java

@@ -1,10 +1,13 @@
 package org.springframework.security.ldap.userdetails;
 
+import java.util.Collection;
+
 import org.springframework.ldap.core.DirContextOperations;
+import org.springframework.security.core.GrantedAuthority;
+import org.springframework.security.core.authority.AuthorityUtils;
 import org.springframework.security.core.userdetails.UserDetails;
 import org.springframework.security.core.userdetails.UserDetailsService;
 import org.springframework.security.core.userdetails.UsernameNotFoundException;
-import org.springframework.security.ldap.authentication.NullLdapAuthoritiesPopulator;
 import org.springframework.security.ldap.search.LdapUserSearch;
 import org.springframework.util.Assert;
 
@@ -16,8 +19,8 @@ import org.springframework.util.Assert;
  * @author Luke Taylor
  */
 public class LdapUserDetailsService implements UserDetailsService {
-    private LdapUserSearch userSearch;
-    private LdapAuthoritiesPopulator authoritiesPopulator;
+    private final LdapUserSearch userSearch;
+    private final LdapAuthoritiesPopulator authoritiesPopulator;
     private UserDetailsContextMapper userDetailsMapper = new LdapUserDetailsMapper();
 
     public LdapUserDetailsService(LdapUserSearch userSearch) {
@@ -42,4 +45,10 @@ public class LdapUserDetailsService implements UserDetailsService {
         Assert.notNull(userDetailsMapper, "userDetailsMapper must not be null");
         this.userDetailsMapper = userDetailsMapper;
     }
+
+    private static final class NullLdapAuthoritiesPopulator implements LdapAuthoritiesPopulator {
+        public Collection<GrantedAuthority> getGrantedAuthorities(DirContextOperations userDetails, String username) {
+            return AuthorityUtils.NO_AUTHORITIES;
+        }
+    }
 }