Explorar o código

Added support for multiple DN patterns. Changes to favour constructor injection for mandatory properties. Renamed LdapUserInfo to prevent confusion with UserDetails interface.

Luke Taylor %!s(int64=20) %!d(string=hai) anos
pai
achega
1f66750e24
Modificáronse 21 ficheiros con 300 adicións e 288 borrados
  1. 47 38
      core/src/main/java/org/acegisecurity/providers/ldap/DefaultInitialDirContextFactory.java
  2. 21 21
      core/src/main/java/org/acegisecurity/providers/ldap/LdapAuthenticationProvider.java
  3. 1 1
      core/src/main/java/org/acegisecurity/providers/ldap/LdapAuthenticator.java
  4. 4 1
      core/src/main/java/org/acegisecurity/providers/ldap/LdapAuthoritiesPopulator.java
  5. 2 6
      core/src/main/java/org/acegisecurity/providers/ldap/LdapUserInfo.java
  6. 18 0
      core/src/main/java/org/acegisecurity/providers/ldap/LdapUtils.java
  7. 52 39
      core/src/main/java/org/acegisecurity/providers/ldap/authenticator/AbstractLdapAuthenticator.java
  8. 21 13
      core/src/main/java/org/acegisecurity/providers/ldap/authenticator/BindAuthenticator.java
  9. 3 3
      core/src/main/java/org/acegisecurity/providers/ldap/authenticator/FilterBasedLdapUserSearch.java
  10. 3 3
      core/src/main/java/org/acegisecurity/providers/ldap/authenticator/LdapUserSearch.java
  11. 21 8
      core/src/main/java/org/acegisecurity/providers/ldap/authenticator/PasswordComparisonAuthenticator.java
  12. 39 19
      core/src/main/java/org/acegisecurity/providers/ldap/populator/DefaultLdapAuthoritiesPopulator.java
  13. 9 22
      core/src/test/java/org/acegisecurity/providers/ldap/InitialDirContextFactoryTests.java
  14. 6 13
      core/src/test/java/org/acegisecurity/providers/ldap/LdapAuthenticationProviderTests.java
  15. 1 29
      core/src/test/java/org/acegisecurity/providers/ldap/LdapTestServer.java
  16. 12 16
      core/src/test/java/org/acegisecurity/providers/ldap/authenticator/BindAuthenticatorTests.java
  17. 6 8
      core/src/test/java/org/acegisecurity/providers/ldap/authenticator/FilterBasedLdapUserSearchTests.java
  18. 4 4
      core/src/test/java/org/acegisecurity/providers/ldap/authenticator/MockUserSearch.java
  19. 8 5
      core/src/test/java/org/acegisecurity/providers/ldap/authenticator/PasswordComparisonAuthenticatorMockTests.java
  20. 18 20
      core/src/test/java/org/acegisecurity/providers/ldap/authenticator/PasswordComparisonAuthenticatorTests.java
  21. 4 19
      core/src/test/java/org/acegisecurity/providers/ldap/populator/DefaultLdapAuthoritiesPopulatorTests.java

+ 47 - 38
core/src/main/java/org/acegisecurity/providers/ldap/DefaultInitialDirContextFactory.java

@@ -25,7 +25,6 @@ import javax.naming.directory.InitialDirContext;
 import javax.naming.directory.DirContext;
 
 import org.springframework.dao.DataAccessResourceFailureException;
-import org.springframework.beans.factory.InitializingBean;
 import org.springframework.util.Assert;
 import org.acegisecurity.BadCredentialsException;
 import org.apache.commons.logging.Log;
@@ -39,7 +38,7 @@ import org.apache.commons.logging.LogFactory;
  * This should be in the form <tt>ldap://monkeymachine.co.uk:389/dc=acegisecurity,dc=org</tt>.
  * </p>
  * <p>
- * To obtain an initial context, th client calls the <tt>newInitialDirContext</tt>
+ * To obtain an initial context, the client calls the <tt>newInitialDirContext</tt>
  * method. There are two signatures - one with no arguments and one which allows
  * binding with a specific username and password.
  * </p>
@@ -53,16 +52,15 @@ import org.apache.commons.logging.LogFactory;
  * as a specific user.
  * </p>
  *
- * @see <a href="http://java.sun.com/products/jndi/tutorial/ldap/connect/pool.html">The Java tutorial's guide to
- * Connection Pooling</a>
+ * @see <a href="http://java.sun.com/products/jndi/tutorial/ldap/connect/pool.html">The Java
+ * tutorial's guide to LDAP connection pooling</a>
  *
  * @author Robert Sanders
  * @author Luke Taylor
  * @version $Id$
  *
  */
-public class DefaultInitialDirContextFactory implements InitialDirContextFactory,
-    InitializingBean {
+public class DefaultInitialDirContextFactory implements InitialDirContextFactory {
     
     //~ Static fields/initializers =============================================
 
@@ -118,13 +116,39 @@ public class DefaultInitialDirContextFactory implements InitialDirContextFactory
      */
     private boolean useConnectionPool = true;    
 
+    //~ Constructors ===========================================================
+
+    public DefaultInitialDirContextFactory(String url) {
+        this.url = url;
+
+        Assert.hasLength(url, "An LDAP connection URL must be supplied.");
+
+        if(url.startsWith("ldap:")) {
+
+            URI uri = LdapUtils.parseLdapUrl(url);
+
+            rootDn = uri.getPath();
+
+        } else {
+            // Assume it's an embedded server
+            rootDn = url;
+        }
+
+        if(rootDn.startsWith("/")) {
+            rootDn = rootDn.substring(1);
+        }
+
+        // This doesn't necessarily hold for embedded servers.
+        //Assert.isTrue(uri.getScheme().equals("ldap"), "Ldap URL must start with 'ldap://'");
+    }
+
     //~ Methods ================================================================
 
     /**
      * Connects anonymously unless a manager user has been specified, in which case
      * it will bind as the manager.
      *
-     * @return the resulting
+     * @return the resulting context object.
      */
     public DirContext newInitialDirContext() {
 
@@ -152,7 +176,8 @@ public class DefaultInitialDirContextFactory implements InitialDirContextFactory
     }
 
     /**
-     * @return The Hashtable describing the base DirContext that will be created, minus the username/password if any.
+     * @return the Hashtable describing the base DirContext that will be created,
+     * minus the username/password if any.
      */
     protected Hashtable getEnvironment() {
         Hashtable env = new Hashtable();
@@ -174,8 +199,15 @@ public class DefaultInitialDirContextFactory implements InitialDirContextFactory
 
     private InitialDirContext connect(Hashtable env) {
         
-// Prints the password, so don't use except for debugging.
-//        logger.debug("Creating initial context with env " + env);
+        if(logger.isDebugEnabled()) {
+            Hashtable envClone = (Hashtable)env.clone();
+
+            if(envClone.containsKey(Context.SECURITY_CREDENTIALS)) {
+                envClone.put(Context.SECURITY_CREDENTIALS, "******");
+            }
+
+            logger.debug("Creating InitialDirContext with environment " + envClone);
+        }
 
         try {
             return new InitialDirContext(env);
@@ -189,27 +221,6 @@ public class DefaultInitialDirContextFactory implements InitialDirContextFactory
         }
     }
 
-    public void afterPropertiesSet() throws Exception {
-        Assert.hasLength(url, "An LDAP connection URL must be supplied.");
-
-        if(url.startsWith("ldap:")) {
-
-            URI uri = new URI(url);
-
-            rootDn = uri.getPath();
-        } else {
-            // Assume it's an embedded server
-            rootDn = url;
-        }
-
-        if(rootDn.startsWith("/")) {
-            rootDn = rootDn.substring(1);
-        }
-
-        //Assert.isTrue(uri.getScheme().equals("ldap"), "Ldap URL must start with 'ldap://'");
-
-    }
-
     /**
      * Returns the root DN of the configured provider URL. For example,
      * if the URL is <tt>ldap://monkeymachine.co.uk:389/dc=acegisecurity,dc=org</tt>
@@ -222,12 +233,12 @@ public class DefaultInitialDirContextFactory implements InitialDirContextFactory
     }
 
     public void setAuthenticationType(String authenticationType) {
-        Assert.hasLength(authenticationType);
+        Assert.hasLength(authenticationType, "LDAP Authentication type must not be empty or null");
         this.authenticationType = authenticationType;
     }
 
     public void setInitialContextFactory(String initialContextFactory) {
-        Assert.hasLength(initialContextFactory);
+        Assert.hasLength(initialContextFactory, "Initial context factory name cannot be empty or null");
         this.initialContextFactory = initialContextFactory;
     }
 
@@ -235,6 +246,7 @@ public class DefaultInitialDirContextFactory implements InitialDirContextFactory
      * @param managerDn The name of the "manager" user for default authentication.
      */
     public void setManagerDn(String managerDn) {
+        Assert.hasLength(managerDn, "Manager user name  cannot be empty or null.");
         this.managerDn = managerDn;
     }
 
@@ -242,18 +254,15 @@ public class DefaultInitialDirContextFactory implements InitialDirContextFactory
      * @param managerPassword The "manager" user's password.
      */
     public void setManagerPassword(String managerPassword) {
+        Assert.hasLength(managerPassword, "Manager password must not be empty or null.");
         this.managerPassword = managerPassword;
     }
 
-    public void setUrl(String url) {
-        this.url = url;
-    }
-
     /**
      * @param extraEnvVars extra environment variables to be added at config time.
      */
     public void setExtraEnvVars(Map extraEnvVars) {
+        Assert.notNull(extraEnvVars, "Extra environment map cannot be null.");
         this.extraEnvVars = extraEnvVars;
     }
-
 }

+ 21 - 21
core/src/main/java/org/acegisecurity/providers/ldap/LdapAuthenticationProvider.java

@@ -76,7 +76,24 @@ public class LdapAuthenticationProvider extends AbstractUserDetailsAuthenticatio
 
     private LdapAuthenticator authenticator;
 
-    private LdapAuthoritiesPopulator ldapAuthoritiesPopulator;
+    private LdapAuthoritiesPopulator authoritiesPopulator;
+
+
+    //~ Constructors ===========================================================
+
+    public LdapAuthenticationProvider(LdapAuthenticator authenticator,
+                                      LdapAuthoritiesPopulator authoritiesPopulator) {
+        Assert.notNull(authenticator, "An LdapAuthenticator must be supplied");
+        Assert.notNull(authoritiesPopulator, "An LdapAuthoritiesPopulator must be supplied");
+
+        this.authenticator = authenticator;
+        this.authoritiesPopulator = authoritiesPopulator;
+
+        // TODO: Check that the role attributes specified for the populator will be retrieved
+        // by the authenticator. If not, add them to the authenticator's list and log a
+        // warning.
+
+    }
 
     //~ Methods ================================================================
 
@@ -89,21 +106,11 @@ public class LdapAuthenticationProvider extends AbstractUserDetailsAuthenticatio
         String password = (String)authentication.getCredentials();
         Assert.notNull(password, "Null password was supplied in authentication token");
 
-        LdapUserDetails ldapUser = authenticator.authenticate(username, password);
+        LdapUserInfo ldapUser = authenticator.authenticate(username, password);
 
         return createUserDetails(username, password, ldapUser.getDn(), ldapUser.getAttributes());
     }
 
-    protected void doAfterPropertiesSet() throws Exception {
-        super.doAfterPropertiesSet();
-        Assert.notNull(authenticator, "An LdapAuthenticator must be supplied");
-        Assert.notNull(ldapAuthoritiesPopulator, "An LdapAuthoritiesPopulator must be supplied");
-
-        // TODO: Check that the role attributes specified for the populator will be retrieved
-        // by the authenticator. If not, add them to the authenticator's list and log a
-        // warning.
-    }
-
     /**
      * Creates the user final <tt>UserDetails</tt> object that will be returned by the provider
      * once the user has been authenticated.
@@ -124,15 +131,8 @@ public class LdapAuthenticationProvider extends AbstractUserDetailsAuthenticatio
     protected UserDetails createUserDetails(String username, String password, String userDn, Attributes attributes) {
 
         return new User(username, password, true, true, true, true,
-                ldapAuthoritiesPopulator.getGrantedAuthorities(username, userDn, attributes));
+                authoritiesPopulator.getGrantedAuthorities(username, userDn, attributes));
 
     }
-
-    public void setAuthenticator(LdapAuthenticator authenticator) {
-        this.authenticator = authenticator;
-    }
-
-    public void setLdapAuthoritiesPopulator(LdapAuthoritiesPopulator ldapAuthoritiesPopulator) {
-        this.ldapAuthoritiesPopulator = ldapAuthoritiesPopulator;
-    }
 }
+

+ 1 - 1
core/src/main/java/org/acegisecurity/providers/ldap/LdapAuthenticator.java

@@ -35,5 +35,5 @@ public interface LdapAuthenticator {
      * @param password the user's password supplied at login.
      * @return the details of the successfully authenticated user.
      */
-    LdapUserDetails authenticate(String username, String password);
+    LdapUserInfo authenticate(String username, String password);
 }

+ 4 - 1
core/src/main/java/org/acegisecurity/providers/ldap/LdapAuthoritiesPopulator.java

@@ -32,12 +32,15 @@ import javax.naming.directory.Attributes;
 public interface LdapAuthoritiesPopulator {
 
     /**
+     * Get the list of authorities for the user.
      *
      * @param username the login name which was passed to the LDAP provider.
      * @param userDn the full DN of the user
      * @param userAttributes the user's LDAP attributes that were retrieved from the directory.
      * @return the granted authorities for the given user.
+     * @throws LdapDataAccessException if there is a problem accessing the directory. 
      */
-    GrantedAuthority[] getGrantedAuthorities(String username, String userDn, Attributes userAttributes);
+    GrantedAuthority[] getGrantedAuthorities(String username, String userDn, Attributes userAttributes)
+            throws LdapDataAccessException;
 
 }

+ 2 - 6
core/src/main/java/org/acegisecurity/providers/ldap/LdapUserDetails.java → core/src/main/java/org/acegisecurity/providers/ldap/LdapUserInfo.java

@@ -15,10 +15,6 @@
 
 package org.acegisecurity.providers.ldap;
 
-import org.acegisecurity.userdetails.User;
-import org.acegisecurity.GrantedAuthority;
-import org.acegisecurity.GrantedAuthorityImpl;
-
 import javax.naming.directory.Attributes;
 import javax.naming.directory.DirContext;
 import javax.naming.NamingException;
@@ -40,7 +36,7 @@ import javax.naming.NamingException;
  * @author Luke Taylor
  * @version $Id$
  */
-public class LdapUserDetails {
+public class LdapUserInfo {
 
     //~ Instance fields ========================================================
 
@@ -54,7 +50,7 @@ public class LdapUserDetails {
      * @param dn the full DN of the user
      * @param attributes any attributes loaded from the user's directory entry.
      */
-    public LdapUserDetails(String dn, Attributes attributes) {
+    public LdapUserInfo(String dn, Attributes attributes) {
         this.dn = dn;
         this.attributes = attributes;
     }

+ 18 - 0
core/src/main/java/org/acegisecurity/providers/ldap/LdapUtils.java

@@ -22,6 +22,8 @@ import org.springframework.util.Assert;
 import javax.naming.Context;
 import javax.naming.NamingException;
 import java.io.UnsupportedEncodingException;
+import java.net.URI;
+import java.net.URISyntaxException;
 
 /**
  * LDAP Utility methods.
@@ -46,6 +48,22 @@ public class LdapUtils {
         }
     }
 
+    /**
+     * Parses the supplied LDAP URL.
+     * @param url the URL (e.g. <tt>ldap://monkeymachine:11389/dc=acegisecurity,dc=org</tt>).
+     * @return the URI object created from the URL
+     * @throws IllegalArgumentException if the URL is null, empty or the URI syntax is invalid.
+     */
+    public static URI parseLdapUrl(String url) {
+        Assert.hasLength(url);
+
+        try {
+            return new URI(url);
+        } catch (URISyntaxException e) {
+            throw new IllegalArgumentException("Unable to parse url: " + url, e);
+        }
+    }
+
     public static byte[] getUtf8Bytes(String s) {
         try {
             return s.getBytes("UTF-8");

+ 52 - 39
core/src/main/java/org/acegisecurity/providers/ldap/authenticator/AbstractLdapAuthenticator.java

@@ -21,6 +21,8 @@ import org.springframework.beans.factory.InitializingBean;
 import org.springframework.util.Assert;
 
 import java.text.MessageFormat;
+import java.util.List;
+import java.util.ArrayList;
 
 /**
  * @author Luke Taylor
@@ -31,36 +33,52 @@ public abstract class AbstractLdapAuthenticator implements LdapAuthenticator,
 
     //~ Instance fields ========================================================
 
-    private String userDnPattern = null;
-    private MessageFormat userDnFormat = null;
+    //private String[] userDnPattern = null;
+    private MessageFormat[] userDnFormat = null;
     private InitialDirContextFactory initialDirContextFactory;
     private LdapUserSearch userSearch;
     private String[] userAttributes = null;
+    private String dnSuffix = "";
+
+    //~ Constructors ===========================================================
+
+    protected AbstractLdapAuthenticator(InitialDirContextFactory initialDirContextFactory) {
+        Assert.notNull(initialDirContextFactory, "initialDirContextFactory must not be null.");
+        this.initialDirContextFactory = initialDirContextFactory;
+
+        String rootDn = initialDirContextFactory.getRootDn();
+
+        if(rootDn.length() > 0) {
+            dnSuffix = "," + rootDn;
+        }
+    }
 
     //~ Methods ================================================================
 
     /**
-     * Returns the DN of the user, worked out from the userDNPattern property.
-     * The returned value includes the root DN of the provider
-     * URL used to configure the <tt>InitialDirContextfactory</tt>.
+     * Builds list of possible DNs for the user, worked out from the
+     * <tt>userDnPatterns</tt> property. The returned value includes the root DN of
+     * the provider URL used to configure the <tt>InitialDirContextfactory</tt>.
+     *
+     * @param username the user's login name
+     * @return the list of possible DN matches, empty if <tt>userDnPatterns</tt> wasn't
+     * set.
      */
-    protected String getUserDn(String username) {
+    protected List getUserDns(String username) {
         if(userDnFormat == null) {
-            return null;
+            return new ArrayList(0);
         }
 
-        String rootDn = initialDirContextFactory.getRootDn();
-        String userDn;
+        List userDns = new ArrayList(userDnFormat.length);
+        String[] args = new String[] {username};
 
         synchronized( userDnFormat ) {
-            userDn = userDnFormat.format(new String[] {username});
-        }
-
-        if(rootDn.length() > 0) {
-            userDn = userDn + "," + rootDn;
+            for(int i=0; i < userDnFormat.length; i++) {
+                userDns.add( userDnFormat[i].format(args) + dnSuffix );
+            }
         }
 
-        return userDn;
+        return userDns;
     }
 
     /**
@@ -69,24 +87,32 @@ public abstract class AbstractLdapAuthenticator implements LdapAuthenticator,
      * The pattern argument {0} will contain the username.
      * An example would be "cn={0},ou=people".
      */
-    public void setUserDnPattern(String dnPattern) {
-        this.userDnPattern = dnPattern;
-        userDnFormat = null;
+    public void setUserDnPatterns(String[] dnPattern) {
+        Assert.notNull(dnPattern, "The array of DN patterns cannot be set to null");
+//        this.userDnPattern = dnPattern;
+        userDnFormat = new MessageFormat[dnPattern.length];
 
-        if(dnPattern != null) {
-            userDnFormat = new MessageFormat(dnPattern);
+        for(int i=0; i < dnPattern.length; i++) {
+            userDnFormat[i] = new MessageFormat(dnPattern[i]);
         }
     }
 
-    public String[] getUserAttributes() {
-        return userAttributes;
+    /**
+     * Sets the user attributes which will be retrieved from the directory.
+     *
+     * @param userAttributes
+     */
+    public void setUserAttributes(String[] userAttributes) {
+        Assert.notNull(userAttributes, "The userAttributes property cannot be set to null");
+        this.userAttributes = userAttributes;
     }
 
-    public String getUserDnPattern() {
-        return userDnPattern;
+    public String[] getUserAttributes() {
+        return userAttributes;
     }
 
     public void setUserSearch(LdapUserSearch userSearch) {
+        Assert.notNull(userSearch, "The userSearch cannot be set to null");
         this.userSearch = userSearch;
     }
 
@@ -94,25 +120,12 @@ public abstract class AbstractLdapAuthenticator implements LdapAuthenticator,
         return userSearch;
     }
 
-    public void setInitialDirContextFactory(InitialDirContextFactory initialDirContextFactory) {
-        this.initialDirContextFactory = initialDirContextFactory;
-    }
-
-    /**
-     * Sets the user attributes which will be retrieved from the directory.
-     * 
-     * @param userAttributes
-     */
-    public void setUserAttributes(String[] userAttributes) {
-        this.userAttributes = userAttributes;
-    }
-
     protected InitialDirContextFactory getInitialDirContextFactory() {
         return initialDirContextFactory;
     }
 
     public void afterPropertiesSet() throws Exception {
-        Assert.notNull(initialDirContextFactory, "initialDirContextFactory must be supplied.");
-        Assert.isTrue(userDnPattern != null || userSearch != null, "Either an LdapUserSearch or DN pattern (or both) must be supplied.");
+        Assert.isTrue(userDnFormat != null || userSearch != null,
+                "Either an LdapUserSearch or DN pattern (or both) must be supplied.");
     }
 }

+ 21 - 13
core/src/main/java/org/acegisecurity/providers/ldap/authenticator/BindAuthenticator.java

@@ -23,6 +23,7 @@ import org.apache.commons.logging.LogFactory;
 import javax.naming.directory.DirContext;
 import javax.naming.directory.Attributes;
 import javax.naming.NamingException;
+import java.util.Iterator;
 
 /**
  * An authenticator which binds as a user.
@@ -32,28 +33,35 @@ import javax.naming.NamingException;
  * @author Luke Taylor
  * @version $Id$
  */
-public class BindAuthenticator extends AbstractLdapAuthenticator {
+public final class BindAuthenticator extends AbstractLdapAuthenticator {
 
     //~ Static fields/initializers =============================================
 
     private static final Log logger = LogFactory.getLog(BindAuthenticator.class);
 
+    //~ Constructors ===========================================================
+
+    public BindAuthenticator(InitialDirContextFactory initialDirContextFactory) {
+        super(initialDirContextFactory);
+    }
+
     //~ Methods ================================================================
 
-    public LdapUserDetails authenticate(String username, String password) {
+    public LdapUserInfo authenticate(String username, String password) {
 
-        String dn = getUserDn(username);
-        LdapUserDetails user = null;
+        LdapUserInfo user = null;
 
-        // If DN is pattern is configured, try authenticating with that directly
-        if(dn != null) {
-            user = authenticateWithDn(dn, password);
+        // If DN patterns are configured, try authenticating with them directly
+        Iterator dns = getUserDns(username).iterator();
+
+        while(dns.hasNext() && user == null) {
+            user = authenticateWithDn((String)dns.next(), password);
         }
 
         // Otherwise use the configured locator to find the user
         // and authenticate with the returned DN.
         if(user == null && getUserSearch() != null) {
-            LdapUserDetails userFromSearch = getUserSearch().searchForUser(username);
+            LdapUserInfo userFromSearch = getUserSearch().searchForUser(username);
             user = authenticateWithDn(userFromSearch.getDn(), password);
         }
 
@@ -65,13 +73,13 @@ public class BindAuthenticator extends AbstractLdapAuthenticator {
 
     }
 
-    private LdapUserDetails authenticateWithDn(String userDn, String password) {
+    private LdapUserInfo authenticateWithDn(String userDn, String password) {
         DirContext ctx = null;
-        LdapUserDetails user = null;
+        LdapUserInfo user = null;
         Attributes attributes = null;
 
         if(logger.isDebugEnabled()) {
-            logger.debug("Binding with DN = " + userDn);
+            logger.debug("Attempting to bind with DN = " + userDn);
         }
 
         try {
@@ -79,8 +87,8 @@ public class BindAuthenticator extends AbstractLdapAuthenticator {
             attributes = ctx.getAttributes(
                     LdapUtils.getRelativeName(userDn, ctx),
                     getUserAttributes());
-            user = new LdapUserDetails(userDn, attributes);
-            
+            user = new LdapUserInfo(userDn, attributes);
+
         } catch(NamingException ne) {
             throw new LdapDataAccessException("Failed to load attributes for user " + userDn, ne);
         } catch(BadCredentialsException e) {

+ 3 - 3
core/src/main/java/org/acegisecurity/providers/ldap/authenticator/FilterBasedLdapUserSearch.java

@@ -84,12 +84,12 @@ public class FilterBasedLdapUserSearch implements LdapUserSearch {
     //~ Methods ================================================================
 
     /**
-     * Return the LdapUserDetails containing the user's information, or null if
+     * Return the LdapUserInfo containing the user's information, or null if
      * no SearchResult is found.
      *
      * @param username the username to search for.
      */
-    public LdapUserDetails searchForUser(String username) {
+    public LdapUserInfo searchForUser(String username) {
         DirContext ctx = initialDirContextFactory.newInitialDirContext();
         SearchControls ctls = new SearchControls();
         ctls.setTimeLimit( searchTimeLimit );
@@ -120,7 +120,7 @@ public class FilterBasedLdapUserSearch implements LdapUserSearch {
             userDn.append(",");
             userDn.append(ctx.getNameInNamespace());
 
-            return new LdapUserDetails(userDn.toString(), searchResult.getAttributes());
+            return new LdapUserInfo(userDn.toString(), searchResult.getAttributes());
 
         } catch(NamingException ne) {
             throw new LdapDataAccessException("User Couldn't be found due to exception", ne);

+ 3 - 3
core/src/main/java/org/acegisecurity/providers/ldap/authenticator/LdapUserSearch.java

@@ -15,7 +15,7 @@
 
 package org.acegisecurity.providers.ldap.authenticator;
 
-import org.acegisecurity.providers.ldap.LdapUserDetails;
+import org.acegisecurity.providers.ldap.LdapUserInfo;
 
 /**
  * Obtains a user's information from the LDAP directory given a login name.
@@ -35,9 +35,9 @@ public interface LdapUserSearch {
      * for that user.
      *
      * @param username the login name supplied to the authentication service.
-     * @return an LdapUserDetails object containing the user's full DN and requested attributes.
+     * @return an LdapUserInfo object containing the user's full DN and requested attributes.
      * TODO: Need to optionally supply required attributes here for the search.
      */
-    LdapUserDetails searchForUser(String username);
+    LdapUserInfo searchForUser(String username);
 
 }

+ 21 - 8
core/src/main/java/org/acegisecurity/providers/ldap/authenticator/PasswordComparisonAuthenticator.java

@@ -15,8 +15,9 @@
 
 package org.acegisecurity.providers.ldap.authenticator;
 
-import org.acegisecurity.providers.ldap.LdapUserDetails;
+import org.acegisecurity.providers.ldap.LdapUserInfo;
 import org.acegisecurity.providers.ldap.LdapUtils;
+import org.acegisecurity.providers.ldap.InitialDirContextFactory;
 import org.acegisecurity.providers.encoding.PasswordEncoder;
 import org.acegisecurity.BadCredentialsException;
 import org.acegisecurity.userdetails.UsernameNotFoundException;
@@ -29,6 +30,7 @@ import javax.naming.NamingException;
 import javax.naming.directory.SearchControls;
 import javax.naming.directory.DirContext;
 import javax.naming.directory.Attribute;
+import java.util.Iterator;
 
 /**
  * An {@link org.acegisecurity.providers.ldap.LdapAuthenticator LdapAuthenticator}
@@ -49,7 +51,7 @@ import javax.naming.directory.Attribute;
  * @author Luke Taylor
  * @version $Id$
  */
-public class PasswordComparisonAuthenticator extends AbstractLdapAuthenticator {
+public final class PasswordComparisonAuthenticator extends AbstractLdapAuthenticator {
     //~ Static fields/initializers =============================================
 
     private static final Log logger = LogFactory.getLog(PasswordComparisonAuthenticator.class);
@@ -64,21 +66,28 @@ public class PasswordComparisonAuthenticator extends AbstractLdapAuthenticator {
 
     private PasswordEncoder passwordEncoder = new LdapShaPasswordEncoder();
 
+    //~ Constructors ===========================================================
+
+    public PasswordComparisonAuthenticator(InitialDirContextFactory initialDirContextFactory) {
+        super(initialDirContextFactory);
+    }
+
     //~ Methods ================================================================
 
-    public LdapUserDetails authenticate(String username, String password) {
+    public LdapUserInfo authenticate(String username, String password) {
 
         // locate the user and check the password
-        String userDn = getUserDn(username);
-        LdapUserDetails user = null;
+        LdapUserInfo user = null;
 
         DirContext ctx = getInitialDirContextFactory().newInitialDirContext();
+        Iterator dns = getUserDns(username).iterator();
 
         try {
-            if(userDn != null) {
+            while(dns.hasNext() && user == null) {
+                String userDn = (String)dns.next();
                 String relativeName = LdapUtils.getRelativeName(userDn, ctx);
 
-                user = new LdapUserDetails(userDn,
+                user = new LdapUserInfo(userDn,
                         ctx.getAttributes(relativeName, getUserAttributes()));
             }
 
@@ -105,6 +114,10 @@ public class PasswordComparisonAuthenticator extends AbstractLdapAuthenticator {
                 }
 
             } else {
+                if(logger.isDebugEnabled()) {
+                    logger.debug("Password attribute " + passwordAttributeName
+                            + " wasn't retrieved for user " + username);
+                }
 
                 doPasswordCompare(ctx, user.getRelativeName(ctx), password);
             }
@@ -153,7 +166,7 @@ public class PasswordComparisonAuthenticator extends AbstractLdapAuthenticator {
     }
 
     public void setPasswordAttributeName(String passwordAttribute) {
-        Assert.hasLength(passwordAttribute, "passwordAttribute must not be empty or null");
+        Assert.hasLength(passwordAttribute, "passwordAttributeName must not be empty or null");
         this.passwordAttributeName = passwordAttribute;
         this.passwordCompareFilter = "(" + passwordAttributeName + "={0})";
     }

+ 39 - 19
core/src/main/java/org/acegisecurity/providers/ldap/populator/DefaultLdapAuthoritiesPopulator.java

@@ -24,7 +24,6 @@ import org.acegisecurity.GrantedAuthorityImpl;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.springframework.util.Assert;
-import org.springframework.beans.factory.InitializingBean;
 
 import javax.naming.directory.Attributes;
 import javax.naming.directory.Attribute;
@@ -85,17 +84,18 @@ import java.util.HashSet;
  * setting the <tt>groupRoleAttribute</tt> property (the default is "cn").
  * </p>
  * <p>
+ * <pre>
  * &lt;bean id="ldapAuthoritiesPopulator" class="org.acegisecurity.providers.ldap.populator.DefaultLdapAuthoritiesPopulator">
  * TODO
  * &lt;/bean>
+ * </pre>
  * </p>
  *
  *
  * @author Luke Taylor
  * @version $Id$
  */
-public class DefaultLdapAuthoritiesPopulator implements LdapAuthoritiesPopulator,
-    InitializingBean {
+public class DefaultLdapAuthoritiesPopulator implements LdapAuthoritiesPopulator {
     //~ Static fields/initializers =============================================
 
     private static final Log logger = LogFactory.getLog(DefaultLdapAuthoritiesPopulator.class);
@@ -105,7 +105,7 @@ public class DefaultLdapAuthoritiesPopulator implements LdapAuthoritiesPopulator
     /** Attributes of the User's LDAP Object that contain role name information. */
     private String[] userRoleAttributes = null;
 
-    private String rolePrefix = "";
+    private String rolePrefix = "ROLE_";
 
     /** The base DN from which the search for group membership should be performed */
     private String groupSearchBase = null;
@@ -127,6 +127,30 @@ public class DefaultLdapAuthoritiesPopulator implements LdapAuthoritiesPopulator
     /** An initial context factory is only required if searching for groups is required. */
     private InitialDirContextFactory initialDirContextFactory = null;
 
+    //~ Constructors ===========================================================
+
+    /**
+     * Constructor for non-group search scenarios. Typically in this case
+     * the <tt>userRoleAttributes</tt> property will be set to obtain roles directly
+     * from the user's directory entry attributes.
+     */
+    public DefaultLdapAuthoritiesPopulator() {
+    }
+
+    /**
+     * Constructor for group search scenarios. <tt>userRoleAttributes</tt> may still be
+     * set as a property.
+     *
+     * @param initialDirContextFactory
+     * @param groupSearchBase
+     */
+    public DefaultLdapAuthoritiesPopulator(InitialDirContextFactory initialDirContextFactory, String groupSearchBase) {
+        Assert.notNull(initialDirContextFactory, "InitialDirContextFactory must not be null");
+        Assert.hasLength(groupSearchBase, "The groupSearchBase (name to search under), must be specified.");
+        this.initialDirContextFactory = initialDirContextFactory;
+        this.groupSearchBase = groupSearchBase;
+    }
+
     //~ Methods ================================================================
 
     /**
@@ -176,6 +200,12 @@ public class DefaultLdapAuthoritiesPopulator implements LdapAuthoritiesPopulator
             return null;
         }
 
+        if(logger.isDebugEnabled()) {
+            logger.debug("Searching for roles for user '"
+                    + userDn + "', with filter "+ groupSearchFilter
+                    + " in search base '" + groupSearchBase + "'");
+        }
+
         DirContext ctx = initialDirContextFactory.newInitialDirContext();
         SearchControls ctls = new SearchControls();
 
@@ -200,11 +230,15 @@ public class DefaultLdapAuthoritiesPopulator implements LdapAuthoritiesPopulator
                 }
             }
         } catch (NamingException e) {
-
+            throw new LdapDataAccessException("Group search failed for user " + userDn, e);
         } finally {
             LdapUtils.closeContext(ctx);
         }
 
+        if(logger.isDebugEnabled()) {
+            logger.debug("Roles from search: " + userRoles);
+        }
+
         return userRoles;
     }
 
@@ -249,10 +283,6 @@ public class DefaultLdapAuthoritiesPopulator implements LdapAuthoritiesPopulator
         this.rolePrefix = rolePrefix;
     }
 
-    public void setGroupSearchBase(String groupSearchBase) {
-        this.groupSearchBase = groupSearchBase;
-    }
-
     public void setGroupSearchFilter(String groupSearchFilter) {
         Assert.notNull(groupSearchFilter, "groupSearchFilter must not be null");
         this.groupSearchFilter = groupSearchFilter;
@@ -272,14 +302,4 @@ public class DefaultLdapAuthoritiesPopulator implements LdapAuthoritiesPopulator
     public void setConvertToUpperCase(boolean convertToUpperCase) {
         this.convertToUpperCase = convertToUpperCase;
     }
-
-    public void setInitialDirContextFactory(InitialDirContextFactory initialDirContextFactory) {
-        this.initialDirContextFactory = initialDirContextFactory;
-    }
-
-    public void afterPropertiesSet() throws Exception {
-        if(initialDirContextFactory == null && groupSearchBase != null) {
-            throw new IllegalArgumentException("initialDirContextFactory is required for group role searches.");
-        }
-    }
 }

+ 9 - 22
core/src/test/java/org/acegisecurity/providers/ldap/InitialDirContextFactoryTests.java

@@ -30,20 +30,19 @@ public class InitialDirContextFactoryTests extends AbstractLdapServerTestCase {
 //    }
 
     public void setUp() {
-        idf = new DefaultInitialDirContextFactory();
+        idf = new DefaultInitialDirContextFactory(PROVIDER_URL);
         idf.setInitialContextFactory(CONTEXT_FACTORY);
         idf.setExtraEnvVars(EXTRA_ENV);
     }
 
     public void testConnectionFailure() throws Exception {
-
-        idf.setInitialContextFactory("com.sun.jndi.ldap.LdapCtxFactory");
         // Use the wrong port
-        idf.setUrl("ldap://localhost:60389");
+        idf = new DefaultInitialDirContextFactory("ldap://localhost:60389");
+        idf.setInitialContextFactory("com.sun.jndi.ldap.LdapCtxFactory");
         Hashtable env = new Hashtable();
         env.put("com.sun.jndi.ldap.connect.timeout", "200");
         idf.setExtraEnvVars(env);
-        idf.afterPropertiesSet();
+
         try {
             idf.newInitialDirContext();
             fail("Connection succeeded unexpectedly");
@@ -52,8 +51,6 @@ public class InitialDirContextFactoryTests extends AbstractLdapServerTestCase {
     }
 
     public void testAnonymousBindSucceeds() throws Exception {
-        idf.setUrl(PROVIDER_URL);
-        idf.afterPropertiesSet();
         DirContext ctx = idf.newInitialDirContext();
         // Connection pooling should be set by default for anon users.
         // Can't rely on this property being there with embedded server
@@ -62,10 +59,9 @@ public class InitialDirContextFactoryTests extends AbstractLdapServerTestCase {
     }
 
     public void testBindAsManagerSucceeds() throws Exception {
-        idf.setUrl(PROVIDER_URL);
         idf.setManagerPassword(MANAGER_PASSWORD);
         idf.setManagerDn(MANAGER_USER);
-        idf.afterPropertiesSet();
+
         DirContext ctx = idf.newInitialDirContext();
 // Can't rely on this property being there with embedded server
 //        assertEquals("true",ctx.getEnvironment().get("com.sun.jndi.ldap.connect.pool"));
@@ -73,10 +69,8 @@ public class InitialDirContextFactoryTests extends AbstractLdapServerTestCase {
     }
 
     public void testInvalidPasswordCausesBadCredentialsException() throws Exception {
-        idf.setUrl(PROVIDER_URL);
         idf.setManagerDn(MANAGER_USER);
         idf.setManagerPassword("wrongpassword");
-        idf.afterPropertiesSet();
         try {
             DirContext ctx = idf.newInitialDirContext();
             fail("Authentication with wrong credentials should fail.");
@@ -85,8 +79,6 @@ public class InitialDirContextFactoryTests extends AbstractLdapServerTestCase {
     }
 
     public void testConnectionAsSpecificUserSucceeds() throws Exception {
-        idf.setUrl(PROVIDER_URL);
-        idf.afterPropertiesSet();
         DirContext ctx = idf.newInitialDirContext("uid=Bob,ou=people,dc=acegisecurity,dc=org",
                 "bobspassword");
         // We don't want pooling for specific users.
@@ -95,7 +87,7 @@ public class InitialDirContextFactoryTests extends AbstractLdapServerTestCase {
     }
 
     public void testEnvironment() {
-        idf.setUrl("ldap://acegisecurity.org/");
+        idf = new DefaultInitialDirContextFactory("ldap://acegisecurity.org/");
 
         // check basic env
         Hashtable env = idf.getEnvironment();
@@ -124,20 +116,15 @@ public class InitialDirContextFactoryTests extends AbstractLdapServerTestCase {
     }
 
     public void testBaseDnIsParsedFromCorrectlyFromUrl() throws Exception {
-        idf.setUrl("ldap://acegisecurity.org/dc=acegisecurity,dc=org");
-        idf.afterPropertiesSet();
+        idf = new DefaultInitialDirContextFactory("ldap://acegisecurity.org/dc=acegisecurity,dc=org");
         assertEquals("dc=acegisecurity,dc=org", idf.getRootDn());
 
         // Check with an empty root
-        idf = new DefaultInitialDirContextFactory();
-        idf.setUrl("ldap://acegisecurity.org/");
-        idf.afterPropertiesSet();
+        idf = new DefaultInitialDirContextFactory("ldap://acegisecurity.org/");
         assertEquals("", idf.getRootDn());
 
         // Empty root without trailing slash
-        idf = new DefaultInitialDirContextFactory();
-        idf.setUrl("ldap://acegisecurity.org");
-        idf.afterPropertiesSet();
+        idf = new DefaultInitialDirContextFactory("ldap://acegisecurity.org");
         assertEquals("", idf.getRootDn());
     }
 

+ 6 - 13
core/src/test/java/org/acegisecurity/providers/ldap/LdapAuthenticationProviderTests.java

@@ -6,11 +6,7 @@ import javax.naming.directory.BasicAttributes;
 import org.acegisecurity.GrantedAuthority;
 import org.acegisecurity.GrantedAuthorityImpl;
 import org.acegisecurity.BadCredentialsException;
-import org.acegisecurity.Authentication;
 import org.acegisecurity.providers.UsernamePasswordAuthenticationToken;
-import org.acegisecurity.providers.ldap.authenticator.FilterBasedLdapUserSearch;
-import org.acegisecurity.providers.ldap.authenticator.BindAuthenticator;
-import org.acegisecurity.providers.ldap.populator.DefaultLdapAuthoritiesPopulator;
 import org.acegisecurity.userdetails.UserDetails;
 
 /**
@@ -30,11 +26,8 @@ public class LdapAuthenticationProviderTests extends AbstractLdapServerTestCase
     }
 
     public void testNormalUsage() throws Exception {
-        LdapAuthenticationProvider ldapProvider = new LdapAuthenticationProvider();
-
-        ldapProvider.setAuthenticator(new MockAuthenticator());
-        ldapProvider.setLdapAuthoritiesPopulator(new MockAuthoritiesPopulator());
-        ldapProvider.afterPropertiesSet();
+        LdapAuthenticationProvider ldapProvider
+                = new LdapAuthenticationProvider(new MockAuthenticator(), new MockAuthoritiesPopulator());
 
         UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("bob","bobspassword");
         UserDetails user = ldapProvider.retrieveUser("bob", token);
@@ -59,7 +52,7 @@ public class LdapAuthenticationProviderTests extends AbstractLdapServerTestCase
         BindAuthenticator authenticator = new BindAuthenticator();
         //PasswordComparisonAuthenticator authenticator = new PasswordComparisonAuthenticator();
         authenticator.setInitialDirContextFactory(dirCtxFactory);
-        //authenticator.setUserDnPattern("cn={0},ou=people");
+        //authenticator.setUserDnPatterns("cn={0},ou=people");
 
         FilterBasedLdapUserSearch userSearch = new FilterBasedLdapUserSearch();
         userSearch.setSearchBase("ou=people");
@@ -78,7 +71,7 @@ public class LdapAuthenticationProviderTests extends AbstractLdapServerTestCase
         populator.setGroupSearchBase("ou=groups");
         populator.afterPropertiesSet();
 
-        ldapProvider.setLdapAuthoritiesPopulator(populator);
+        ldapProvider.setAuthoritiesPopulator(populator);
         ldapProvider.setAuthenticator(authenticator);
         Authentication auth = ldapProvider.authenticate(new UsernamePasswordAuthenticationToken("Ben Alex","benspassword"));
         assertEquals(2, auth.getAuthorities().length);
@@ -94,10 +87,10 @@ public class LdapAuthenticationProviderTests extends AbstractLdapServerTestCase
     class MockAuthenticator implements LdapAuthenticator {
         Attributes userAttributes = new BasicAttributes("cn","bob");
 
-        public LdapUserDetails authenticate(String username, String password) {
+        public LdapUserInfo authenticate(String username, String password) {
             if(username.equals("bob") && password.equals("bobspassword")) {
 
-                return new LdapUserDetails("cn=bob,ou=people,dc=acegisecurity,dc=org", userAttributes);
+                return new LdapUserInfo("cn=bob,ou=people,dc=acegisecurity,dc=org", userAttributes);
             }
             throw new BadCredentialsException("Authentication of Bob failed.");
         }

+ 1 - 29
core/src/test/java/org/acegisecurity/providers/ldap/LdapTestServer.java

@@ -90,33 +90,6 @@ public class LdapTestServer {
         }
     }
 
-
-//    private void startLdapServer() {
-//        ApplicationContext factory = new ClassPathXmlApplicationContext( "org/acegisecurity/providers/ldap/apacheds-context.xml");
-//        MutableServerStartupConfiguration cfg = ( MutableServerStartupConfiguration ) factory.getBean( "configuration" );
-//        ClassPathResource ldifDir = new ClassPathResource("org/acegisecurity/providers/ldap/ldif");
-//
-//        try {
-//            cfg.setLdifDirectory(ldifDir.getFile());
-//        } catch (IOException e) {
-//            System.err.println("Failed to set LDIF directory for server");
-//            e.printStackTrace();
-//        }
-//
-//        Properties env = ( Properties ) factory.getBean( "environment" );
-//
-//        env.setProperty( Context.PROVIDER_URL, "dc=acegisecurity,dc=org" );
-//        env.setProperty( Context.INITIAL_CONTEXT_FACTORY, ServerContextFactory.class.getName() );
-//        env.putAll( cfg.toJndiEnvironment() );
-//
-//        try {
-//            serverContext = new InitialDirContext( env );
-//        } catch (NamingException e) {
-//            System.err.println("Failed to start Apache DS");
-//            e.printStackTrace();
-//        }
-//    }
-
     private void initTestData() {
         createOu("people");
         createOu("groups");
@@ -125,7 +98,7 @@ public class LdapTestServer {
         String[] developers = new String[]
                 {"uid=ben,ou=people,dc=acegisecurity,dc=org", "uid=bob,ou=people,dc=acegisecurity,dc=org"};
         createGroup("developers","developer",developers);
-        createGroup("managers","manager",new String[] { developers[0]});
+        createGroup("managers","manager", new String[] { developers[0]});
     }
 
     private void createManagerUser() {
@@ -257,5 +230,4 @@ public class LdapTestServer {
         LdapTestServer server = new LdapTestServer(false);
     }
 
-
 }

+ 12 - 16
core/src/test/java/org/acegisecurity/providers/ldap/authenticator/BindAuthenticatorTests.java

@@ -1,12 +1,12 @@
 package org.acegisecurity.providers.ldap.authenticator;
 
 import org.acegisecurity.providers.ldap.DefaultInitialDirContextFactory;
-import org.acegisecurity.providers.ldap.LdapUserDetails;
+import org.acegisecurity.providers.ldap.LdapUserInfo;
 import org.acegisecurity.providers.ldap.AbstractLdapServerTestCase;
 import org.acegisecurity.BadCredentialsException;
 
 /**
- * Tests {@link BindAuthenticator}.
+ * Tests for {@link BindAuthenticator}.
  *
  * @author Luke Taylor
  * @version $Id$
@@ -17,30 +17,26 @@ public class BindAuthenticatorTests extends AbstractLdapServerTestCase {
     private BindAuthenticator authenticator;
 
     public void setUp() throws Exception {
-        dirCtxFactory = new DefaultInitialDirContextFactory();
+        dirCtxFactory = new DefaultInitialDirContextFactory(PROVIDER_URL);
         dirCtxFactory.setInitialContextFactory(CONTEXT_FACTORY);
         dirCtxFactory.setExtraEnvVars(EXTRA_ENV);
-        dirCtxFactory.setUrl(PROVIDER_URL);
-        dirCtxFactory.afterPropertiesSet();
-        authenticator = new BindAuthenticator();
-        authenticator.setInitialDirContextFactory(dirCtxFactory);
+        authenticator = new BindAuthenticator(dirCtxFactory);
     }
 
     public void testUserDnPatternReturnsCorrectDn() throws Exception {
-        authenticator.setUserDnPattern("cn={0},ou=people");
-        assertEquals("cn=Joe,ou=people,"+ ROOT_DN, authenticator.getUserDn("Joe"));
+        authenticator.setUserDnPatterns(new String[] {"cn={0},ou=people"});
+        assertEquals("cn=Joe,ou=people,"+ ROOT_DN, authenticator.getUserDns("Joe").get(0));
     }
 
     public void testAuthenticationWithCorrectPasswordSucceeds() throws Exception {
-        authenticator.setUserDnPattern("uid={0},ou=people");
-        LdapUserDetails user = authenticator.authenticate("bob","bobspassword");
+        authenticator.setUserDnPatterns(new String[] {"uid={0},ou=people"});
+        LdapUserInfo user = authenticator.authenticate("bob","bobspassword");
     }
 
     public void testAuthenticationWithWrongPasswordFails() {
-        BindAuthenticator authenticator = new BindAuthenticator();
+        BindAuthenticator authenticator = new BindAuthenticator(dirCtxFactory);
 
-        authenticator.setInitialDirContextFactory(dirCtxFactory);
-        authenticator.setUserDnPattern("uid={0},ou=people");
+        authenticator.setUserDnPatterns(new String[] {"uid={0},ou=people"});
 
         try {
             authenticator.authenticate("bob","wrongpassword");
@@ -50,7 +46,7 @@ public class BindAuthenticatorTests extends AbstractLdapServerTestCase {
     }
 
     public void testAuthenticationWithUserSearch() throws Exception {
-        LdapUserDetails user = new LdapUserDetails("uid=bob,ou=people," + ROOT_DN, null);
+        LdapUserInfo user = new LdapUserInfo("uid=bob,ou=people," + ROOT_DN, null);
         authenticator.setUserSearch(new MockUserSearch(user));
         authenticator.afterPropertiesSet();
         authenticator.authenticate("bob","bobspassword");
@@ -63,7 +59,7 @@ public class BindAuthenticatorTests extends AbstractLdapServerTestCase {
 //        BindAuthenticator authenticator = new BindAuthenticator();
 //
 //        authenticator.setInitialDirContextFactory(dirCtxFactory);
-//        authenticator.setUserDnPattern("cn={0},ou=people");
+//        authenticator.setUserDnPatterns("cn={0},ou=people");
 //        try {
 //            authenticator.authenticate("Baz","bobspassword");
 //            fail("Shouldn't be able to bind with invalid username");

+ 6 - 8
core/src/test/java/org/acegisecurity/providers/ldap/authenticator/FilterBasedLdapUserSearchTests.java

@@ -2,7 +2,7 @@ package org.acegisecurity.providers.ldap.authenticator;
 
 import org.acegisecurity.providers.ldap.AbstractLdapServerTestCase;
 import org.acegisecurity.providers.ldap.DefaultInitialDirContextFactory;
-import org.acegisecurity.providers.ldap.LdapUserDetails;
+import org.acegisecurity.providers.ldap.LdapUserInfo;
 import org.acegisecurity.userdetails.UsernameNotFoundException;
 import org.acegisecurity.BadCredentialsException;
 
@@ -17,13 +17,11 @@ public class FilterBasedLdapUserSearchTests extends AbstractLdapServerTestCase {
     private FilterBasedLdapUserSearch locator;
 
     public void setUp() throws Exception {
-        dirCtxFactory = new DefaultInitialDirContextFactory();
+        dirCtxFactory = new DefaultInitialDirContextFactory(PROVIDER_URL);
         dirCtxFactory.setInitialContextFactory(CONTEXT_FACTORY);
         dirCtxFactory.setExtraEnvVars(EXTRA_ENV);
-        dirCtxFactory.setUrl(PROVIDER_URL);
         dirCtxFactory.setManagerDn(MANAGER_USER);
         dirCtxFactory.setManagerPassword(MANAGER_PASSWORD);
-        dirCtxFactory.afterPropertiesSet();
         locator = new FilterBasedLdapUserSearch();
         locator.setSearchSubtree(false);
         locator.setSearchTimeLimit(0);
@@ -42,7 +40,7 @@ public class FilterBasedLdapUserSearchTests extends AbstractLdapServerTestCase {
         locator.setSearchBase("ou=people");
         locator.setSearchFilter("(uid={0})");
         locator.afterPropertiesSet();
-        LdapUserDetails bob = locator.searchForUser("bob");
+        LdapUserInfo bob = locator.searchForUser("bob");
         // name is wrong with embedded apacheDS
 //        assertEquals("uid=bob,ou=people,"+ROOT_DN, bob.getDn());
     }
@@ -52,7 +50,7 @@ public class FilterBasedLdapUserSearchTests extends AbstractLdapServerTestCase {
         locator.setSearchFilter("(cn={0})");
         locator.setSearchSubtree(true);
         locator.afterPropertiesSet();
-        LdapUserDetails ben = locator.searchForUser("Ben Alex");
+        LdapUserInfo ben = locator.searchForUser("Ben Alex");
 //        assertEquals("uid=ben,ou=people,"+ROOT_DN, bob.getDn());
     }
 
@@ -82,10 +80,10 @@ public class FilterBasedLdapUserSearchTests extends AbstractLdapServerTestCase {
 
     public void testExtraFilterPartToExcludeBob() throws Exception {
         locator.setSearchBase("ou=people");
-        locator.setSearchFilter("(&(cn=*)(!(uid={0})))");
+        locator.setSearchFilter("(&(cn=*)(!(|(uid={0})(uid=marissa))))");
 
         // Search for bob, get back ben...
-        LdapUserDetails ben = locator.searchForUser("bob");
+        LdapUserInfo ben = locator.searchForUser("bob");
         String cn = (String)ben.getAttributes().get("cn").get();
         assertEquals("Ben Alex", cn);
 //        assertEquals("uid=ben,ou=people,"+ROOT_DN, ben.getDn());

+ 4 - 4
core/src/test/java/org/acegisecurity/providers/ldap/authenticator/MockUserSearch.java

@@ -1,19 +1,19 @@
 package org.acegisecurity.providers.ldap.authenticator;
 
-import org.acegisecurity.providers.ldap.LdapUserDetails;
+import org.acegisecurity.providers.ldap.LdapUserInfo;
 
 /**
  * @author Luke Taylor
  * @version $Id$
  */
 public class MockUserSearch implements LdapUserSearch {
-    LdapUserDetails user;
+    LdapUserInfo user;
 
-    public MockUserSearch(LdapUserDetails user) {
+    public MockUserSearch(LdapUserInfo user) {
         this.user = user;
     }
 
-    public LdapUserDetails searchForUser(String username) {
+    public LdapUserInfo searchForUser(String username) {
         return user;
     }
 }

+ 8 - 5
core/src/test/java/org/acegisecurity/providers/ldap/authenticator/PasswordComparisonAuthenticatorMockTests.java

@@ -17,11 +17,14 @@ public class PasswordComparisonAuthenticatorMockTests extends MockObjectTestCase
     public void testLdapCompareIsUsedWhenPasswordIsNotRetrieved() throws Exception {
         Mock mockCtx = new Mock(DirContext.class);
 
-        PasswordComparisonAuthenticator authenticator = new PasswordComparisonAuthenticator();
-        authenticator.setUserDnPattern("cn={0},ou=people");
-        authenticator.setInitialDirContextFactory(
-                new MockInitialDirContextFactory((DirContext)mockCtx.proxy(),
-                        "dc=acegisecurity,dc=org"));
+        PasswordComparisonAuthenticator authenticator =
+                new PasswordComparisonAuthenticator(new MockInitialDirContextFactory(
+                        (DirContext)mockCtx.proxy(),
+                        "dc=acegisecurity,dc=org")
+                );
+
+        authenticator.setUserDnPatterns(new String[] {"cn={0},ou=people"});
+
         // Get the mock to return an empty attribute set
         mockCtx.expects(atLeastOnce()).method("getNameInNamespace").will(returnValue("dc=acegisecurity,dc=org"));
         mockCtx.expects(once()).method("getAttributes").with(eq("cn=Bob,ou=people"), NULL).will(returnValue(new BasicAttributes()));

+ 18 - 20
core/src/test/java/org/acegisecurity/providers/ldap/authenticator/PasswordComparisonAuthenticatorTests.java

@@ -1,15 +1,16 @@
 package org.acegisecurity.providers.ldap.authenticator;
 
 import org.acegisecurity.providers.ldap.DefaultInitialDirContextFactory;
-import org.acegisecurity.providers.ldap.LdapUserDetails;
+import org.acegisecurity.providers.ldap.LdapUserInfo;
 import org.acegisecurity.providers.ldap.AbstractLdapServerTestCase;
-import org.acegisecurity.providers.encoding.PlaintextPasswordEncoder;
 import org.acegisecurity.BadCredentialsException;
 import org.acegisecurity.userdetails.UsernameNotFoundException;
 
 import javax.naming.directory.BasicAttributes;
 
 /**
+ * Tests for {@link PasswordComparisonAuthenticator}.
+ *
  * @author Luke Taylor
  * @version $Id$
  */
@@ -18,17 +19,13 @@ public class PasswordComparisonAuthenticatorTests extends AbstractLdapServerTest
     private PasswordComparisonAuthenticator authenticator;
 
     public void setUp() throws Exception {
-        // Connection information
-        dirCtxFactory = new DefaultInitialDirContextFactory();
+        dirCtxFactory = new DefaultInitialDirContextFactory(PROVIDER_URL);
         dirCtxFactory.setInitialContextFactory(CONTEXT_FACTORY);
         dirCtxFactory.setExtraEnvVars(EXTRA_ENV);
-        dirCtxFactory.setUrl(PROVIDER_URL);
         dirCtxFactory.setManagerDn(MANAGER_USER);
         dirCtxFactory.setManagerPassword(MANAGER_PASSWORD);
-        dirCtxFactory.afterPropertiesSet();
-        authenticator = new PasswordComparisonAuthenticator();
-        authenticator.setInitialDirContextFactory(dirCtxFactory);
-        authenticator.setUserDnPattern("uid={0},ou=people");
+        authenticator = new PasswordComparisonAuthenticator(dirCtxFactory);
+        authenticator.setUserDnPatterns(new String[] {"uid={0},ou=people"});
     }
 
     public void tearDown() {
@@ -46,7 +43,7 @@ public class PasswordComparisonAuthenticatorTests extends AbstractLdapServerTest
     public void testLdapCompareSucceedsWithShaEncodedPassword() {
         authenticator = new PasswordComparisonAuthenticator();
         authenticator.setInitialDirContextFactory(dirCtxFactory);
-        authenticator.setUserDnPattern("uid={0},ou=people");
+        authenticator.setUserDnPatterns("uid={0},ou=people");
         // Don't retrieve the password
         authenticator.setUserAttributes(new String[] {"cn"});
         authenticator.authenticate("ben", "benspassword");
@@ -76,9 +73,8 @@ public class PasswordComparisonAuthenticatorTests extends AbstractLdapServerTest
     }
 
     public void testLocalCompareSucceedsWithShaEncodedPassword() {
-        authenticator = new PasswordComparisonAuthenticator();
-        authenticator.setInitialDirContextFactory(dirCtxFactory);
-        authenticator.setUserDnPattern("uid={0},ou=people");
+        authenticator = new PasswordComparisonAuthenticator(dirCtxFactory);
+        authenticator.setUserDnPatterns(new String[] {"uid={0},ou=people"});
         authenticator.authenticate("ben", "benspassword");
     }
 
@@ -91,7 +87,7 @@ public class PasswordComparisonAuthenticatorTests extends AbstractLdapServerTest
     }
 
     public void testAllAttributesAreRetrivedByDefault() {
-        LdapUserDetails user = authenticator.authenticate("Bob", "bobspassword");
+        LdapUserInfo user = authenticator.authenticate("Bob", "bobspassword");
         System.out.println(user.getAttributes().toString());
         assertEquals("User should have 5 attributes", 5, user.getAttributes().size());
 
@@ -100,7 +96,7 @@ public class PasswordComparisonAuthenticatorTests extends AbstractLdapServerTest
     public void testOnlySpecifiedAttributesAreRetrieved() throws Exception {
         authenticator.setUserAttributes(new String[] {"cn", "uid"});
         authenticator.setPasswordEncoder(new PlaintextPasswordEncoder());
-        LdapUserDetails user = authenticator.authenticate("Bob", "bobspassword");
+        LdapUserInfo user = authenticator.authenticate("Bob", "bobspassword");
         assertEquals("Should have retrieved 2 attributes (cn, uid)",2, user.getAttributes().size());
         assertEquals("Bob Hamilton", user.getAttributes().get("cn").get());
         assertEquals("bob", user.getAttributes().get("uid").get());
@@ -120,17 +116,19 @@ public class PasswordComparisonAuthenticatorTests extends AbstractLdapServerTest
  */
 
     public void testWithUserSearch() {
-        LdapUserDetails user = new LdapUserDetails("uid=Bob,ou=people" + ROOT_DN,
+        authenticator = new PasswordComparisonAuthenticator(dirCtxFactory);
+        assertTrue("User DN matches shouldn't be available",
+                authenticator.getUserDns("Bob").isEmpty());
+        LdapUserInfo user = new LdapUserInfo("uid=Bob,ou=people" + ROOT_DN,
                 new BasicAttributes("userPassword","bobspassword"));
-        authenticator.setUserDnPattern(null);
-        assertNull(authenticator.getUserDnPattern());
-        assertNull(authenticator.getUserDn("Bob"));
         authenticator.setUserSearch(new MockUserSearch(user));
         authenticator.authenticate("ShouldntBeUsed","bobspassword");
     }
 
     public void testFailedSearchGivesUserNotFoundException() throws Exception {
-        authenticator.setUserDnPattern(null);
+        authenticator = new PasswordComparisonAuthenticator(dirCtxFactory);
+        assertTrue("User DN matches shouldn't be available",
+                authenticator.getUserDns("Bob").isEmpty());
         authenticator.setUserSearch(new MockUserSearch(null));
         authenticator.afterPropertiesSet();
 

+ 4 - 19
core/src/test/java/org/acegisecurity/providers/ldap/populator/DefaultLdapAuthoritiesPopulatorTests.java

@@ -17,31 +17,17 @@ import java.util.HashSet;
  */
 public class DefaultLdapAuthoritiesPopulatorTests extends AbstractLdapServerTestCase {
     private DefaultInitialDirContextFactory dirCtxFactory;
-    private DefaultLdapAuthoritiesPopulator populator;
 
     public void setUp() {
-        dirCtxFactory = new DefaultInitialDirContextFactory();
-        dirCtxFactory.setUrl(PROVIDER_URL);
+        dirCtxFactory = new DefaultInitialDirContextFactory(PROVIDER_URL);
         dirCtxFactory.setInitialContextFactory(CONTEXT_FACTORY);
         dirCtxFactory.setExtraEnvVars(EXTRA_ENV);
         dirCtxFactory.setManagerDn(MANAGER_USER);
         dirCtxFactory.setManagerPassword(MANAGER_PASSWORD);
-
-        populator = new DefaultLdapAuthoritiesPopulator();
-        populator.setRolePrefix("ROLE_");
-    }
-
-    public void testCtxFactoryMustBeSetIfSearchBaseIsSet() throws Exception {
-        populator.setGroupSearchBase("");
-
-        try {
-            populator.afterPropertiesSet();
-            fail("expected exception.");
-        } catch (IllegalArgumentException expected) {
-        }
     }
 
     public void testUserAttributeMappingToRoles() {
+        DefaultLdapAuthoritiesPopulator populator = new DefaultLdapAuthoritiesPopulator();
         populator.setUserRoleAttributes(new String[] {"userRole", "otherUserRole"});
         populator.getUserRoleAttributes();
 
@@ -58,14 +44,13 @@ public class DefaultLdapAuthoritiesPopulatorTests extends AbstractLdapServerTest
     }
 
     public void testGroupSearch() throws Exception {
-        populator.setInitialDirContextFactory(dirCtxFactory);
-        populator.setGroupSearchBase("ou=groups");
+        DefaultLdapAuthoritiesPopulator populator = new DefaultLdapAuthoritiesPopulator(dirCtxFactory, "ou=groups");
+        populator.setRolePrefix("ROLE_");
         populator.setGroupRoleAttribute("ou");
         populator.setSearchSubtree(true);
         populator.setSearchSubtree(false);
         populator.setConvertToUpperCase(true);
         populator.setGroupSearchFilter("(member={0})");
-        populator.afterPropertiesSet();
 
         GrantedAuthority[] authorities = populator.getGrantedAuthorities("ben", "uid=ben,ou=people,"+ROOT_DN, new BasicAttributes());
         assertEquals("Should have 2 roles", 2, authorities.length);