Browse Source

SEC-449: Mostly changes to aid moving towards compatibility with spring-ldap.

Luke Taylor 18 years ago
parent
commit
9b71b5aa00

+ 30 - 6
core/src/main/java/org/acegisecurity/ldap/DefaultInitialDirContextFactory.java

@@ -26,6 +26,10 @@ import org.springframework.context.MessageSourceAware;
 import org.springframework.context.support.MessageSourceAccessor;
 
 import org.springframework.util.Assert;
+import org.springframework.ldap.ContextSource;
+import org.springframework.ldap.UncategorizedLdapException;
+import org.springframework.ldap.support.DefaultDirObjectFactory;
+import org.springframework.dao.DataAccessException;
 
 import java.util.Hashtable;
 import java.util.Map;
@@ -49,12 +53,12 @@ import javax.naming.directory.InitialDirContext;
  * The Sun JNDI provider also supports lists of space-separated URLs, each of which will be tried in turn until a
  * connection is obtained.
  * </p>
- * <p>To obtain an initial context, the client calls the <tt>newInitialDirContext</tt> method. There are two
+ *  <p>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>
- * <p>The no-args version will bind anonymously unless a manager login has been configured using the properties
+ *  <p>The no-args version will bind anonymously unless a manager login has been configured using the properties
  * <tt>managerDn</tt> and <tt>managerPassword</tt>, in which case it will bind as the manager user.</p>
- * <p>Connection pooling is enabled by default for anonymous or manager connections, but not when binding as a
+ *  <p>Connection pooling is enabled by default for anonymous or manager connections, but not when binding as a
  * specific user.</p>
  *
  * @author Robert Sanders
@@ -64,7 +68,7 @@ import javax.naming.directory.InitialDirContext;
  * @see <a href="http://java.sun.com/products/jndi/tutorial/ldap/connect/pool.html">The Java tutorial's guide to LDAP
  *      connection pooling</a>
  */
-public class DefaultInitialDirContextFactory implements InitialDirContextFactory, MessageSourceAware {
+public class DefaultInitialDirContextFactory implements InitialDirContextFactory, MessageSourceAware, ContextSource {
     //~ Static fields/initializers =====================================================================================
 
     private static final Log logger = LogFactory.getLog(DefaultInitialDirContextFactory.class);
@@ -86,6 +90,8 @@ public class DefaultInitialDirContextFactory implements InitialDirContextFactory
      */
     private String initialContextFactory = "com.sun.jndi.ldap.LdapCtxFactory";
 
+    private String dirObjectFactoryClass = DefaultDirObjectFactory.class.getName();
+
     /**
      * If your LDAP server does not allow anonymous searches then you will need to provide a "manager" user's
      * DN to log in with.
@@ -186,11 +192,11 @@ public class DefaultInitialDirContextFactory implements InitialDirContextFactory
             }
 
             if (ne instanceof CommunicationException) {
-                throw new LdapDataAccessException(messages.getMessage(
+                throw new UncategorizedLdapException(messages.getMessage(
                         "DefaultIntitalDirContextFactory.communicationFailure", "Unable to connect to LDAP server"), ne);
             }
 
-            throw new LdapDataAccessException(messages.getMessage(
+            throw new UncategorizedLdapException(messages.getMessage(
                     "DefaultIntitalDirContextFactory.unexpectedException",
                     "Failed to obtain InitialDirContext due to unexpected exception"), ne);
         }
@@ -258,9 +264,23 @@ public class DefaultInitialDirContextFactory implements InitialDirContextFactory
         env.put(Context.SECURITY_PRINCIPAL, username);
         env.put(Context.SECURITY_CREDENTIALS, password);
 
+        if(dirObjectFactoryClass != null) {
+            env.put(Context.OBJECT_FACTORIES, dirObjectFactoryClass);
+        }
+
         return connect(env);
     }
 
+    /** Spring LDAP <tt>ContextSource</tt> method */
+    public DirContext getReadOnlyContext() throws DataAccessException {
+        return newInitialDirContext();
+    }
+
+    /** Spring LDAP <tt>ContextSource</tt> method */
+    public DirContext getReadWriteContext() throws DataAccessException {
+        return newInitialDirContext();
+    }
+
     public void setAuthenticationType(String authenticationType) {
         Assert.hasLength(authenticationType, "LDAP Authentication type must not be empty or null");
         this.authenticationType = authenticationType;
@@ -321,4 +341,8 @@ public class DefaultInitialDirContextFactory implements InitialDirContextFactory
     public void setUseLdapContext(boolean useLdapContext) {
         this.useLdapContext = useLdapContext;
     }
+
+    public void setDirObjectFactory(String dirObjectFactory) {
+        this.dirObjectFactoryClass = dirObjectFactory;
+    }
 }

+ 3 - 1
core/src/main/java/org/acegisecurity/ldap/InitialDirContextFactory.java

@@ -15,6 +15,8 @@
 
 package org.acegisecurity.ldap;
 
+import org.springframework.ldap.ContextSource;
+
 import javax.naming.directory.DirContext;
 
 
@@ -26,7 +28,7 @@ import javax.naming.directory.DirContext;
  * @author Luke Taylor
  * @version $Id$
  */
-public interface InitialDirContextFactory {
+public interface InitialDirContextFactory extends ContextSource {
     //~ Methods ========================================================================================================
 
     /**

+ 3 - 0
core/src/main/java/org/acegisecurity/ldap/LdapCallback.java

@@ -22,6 +22,9 @@ import javax.naming.directory.DirContext;
 /**
  * Callback object for use with LdapTemplate.
  *
+ * @deprecated use spring-ldap ContextExecutor instead.
+ * @TODO: Delete before 2.0 release
+ *
  * @author Ben Alex
  */
 public interface LdapCallback {

+ 1 - 0
core/src/main/java/org/acegisecurity/ldap/LdapDataAccessException.java

@@ -22,6 +22,7 @@ import org.springframework.dao.DataAccessException;
  * Used to wrap unexpected NamingExceptions while accessing the LDAP server or for other LDAP-related data problems
  * such as data we can't handle.
  *
+ * @deprecated Spring LDAP classes are now used instead.
  * @author Luke Taylor
  * @version $Id$
  */

+ 1 - 0
core/src/main/java/org/acegisecurity/ldap/LdapEntryMapper.java

@@ -24,6 +24,7 @@ import javax.naming.directory.Attributes;
  * a set of attributes retrieved from a directory entry.
  *
  * @author Luke Taylor
+ * @deprecated in favour of Spring LDAP ContextMapper
  * @version $Id$
  */
 public interface LdapEntryMapper {

+ 55 - 59
core/src/main/java/org/acegisecurity/ldap/LdapTemplate.java

@@ -20,6 +20,11 @@ import org.springframework.dao.IncorrectResultSizeDataAccessException;
 
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
+import org.springframework.ldap.ContextSource;
+import org.springframework.ldap.ContextExecutor;
+import org.springframework.ldap.ContextMapper;
+import org.springframework.ldap.support.DirContextAdapter;
+import org.springframework.ldap.support.DistinguishedName;
 
 import java.util.HashSet;
 import java.util.Set;
@@ -36,52 +41,35 @@ import javax.naming.directory.SearchResult;
 
 
 /**
- * LDAP equivalent of the Spring JdbcTemplate class.<p>This is mainly intended to simplify Ldap access within Acegi
- * Security's LDAP-related services.</p>
+ * LDAP equivalent of the Spring JdbcTemplate class.
+ * <p>
+ * This is mainly intended to simplify Ldap access within Acegi Security's LDAP-related services.
+ * </p>
  *
  * @author Ben Alex
  * @author Luke Taylor
  */
-public class LdapTemplate {
+public class LdapTemplate extends org.springframework.ldap.LdapTemplate {
     //~ Static fields/initializers =====================================================================================
 
     public static final String[] NO_ATTRS = new String[0];
 
     //~ Instance fields ================================================================================================
 
-    private InitialDirContextFactory dirContextFactory;
     private NamingExceptionTranslator exceptionTranslator = new LdapExceptionTranslator();
 
     /** Default search controls */
     private SearchControls searchControls = new SearchControls();
-    private String password = null;
-    private String principalDn = null;
 
     //~ Constructors ===================================================================================================
 
-    public LdapTemplate(InitialDirContextFactory dirContextFactory) {
-        Assert.notNull(dirContextFactory, "An InitialDirContextFactory is required");
-        this.dirContextFactory = dirContextFactory;
+    public LdapTemplate(ContextSource contextSource) {
+        Assert.notNull(contextSource, "ContextSource cannot be null");
+        setContextSource(contextSource);
 
         searchControls.setSearchScope(SearchControls.SUBTREE_SCOPE);
     }
 
-/**
-     *
-     * @param dirContextFactory the source of DirContexts
-     * @param userDn the user name to authenticate as when obtaining new contexts
-     * @param password the user's password
-     */
-    public LdapTemplate(InitialDirContextFactory dirContextFactory, String userDn, String password) {
-        this(dirContextFactory);
-
-        Assert.hasLength(userDn, "userDn must not be null or empty");
-        Assert.notNull(password, "password cannot be null");
-
-        this.principalDn = userDn;
-        this.password = password;
-    }
-
     //~ Methods ========================================================================================================
 
     /**
@@ -96,9 +84,9 @@ public class LdapTemplate {
     public boolean compare(final String dn, final String attributeName, final Object value) {
         final String comparisonFilter = "(" + attributeName + "={0})";
 
-        class LdapCompareCallback implements LdapCallback {
-            public Object doInDirContext(DirContext ctx)
-                throws NamingException {
+        class LdapCompareCallback implements ContextExecutor {
+
+            public Object executeWithContext(DirContext ctx) throws NamingException {
                 SearchControls ctls = new SearchControls();
                 ctls.setReturningAttributes(NO_ATTRS);
                 ctls.setSearchScope(SearchControls.OBJECT_SCOPE);
@@ -111,30 +99,28 @@ public class LdapTemplate {
             }
         }
 
-        Boolean matches = (Boolean) execute(new LdapCompareCallback());
+        Boolean matches = (Boolean) executeReadOnly(new LdapCompareCallback());
 
         return matches.booleanValue();
     }
 
-    public Object execute(LdapCallback callback) throws DataAccessException {
-        DirContext ctx = null;
-
-        try {
-            ctx = (principalDn == null) ? dirContextFactory.newInitialDirContext()
-                                        : dirContextFactory.newInitialDirContext(principalDn, password);
-
-            return callback.doInDirContext(ctx);
-        } catch (NamingException exception) {
-            throw exceptionTranslator.translate("LdapCallback", exception);
-        } finally {
-            LdapUtils.closeContext(ctx);
-        }
-    }
+//    public Object execute(LdapCallback callback) throws DataAccessException {
+//        DirContext ctx = null;
+//
+//        try {
+//            ctx = dirContextFactory.getReadOnlyContext();
+//
+//            return callback.doInDirContext(ctx);
+//        } catch (NamingException exception) {
+//            throw exceptionTranslator.translate("LdapCallback", exception);
+//        } finally {
+//            LdapUtils.closeContext(ctx);
+//        }
+//    }
 
     public boolean nameExists(final String dn) {
-        Boolean exists = (Boolean) execute(new LdapCallback() {
-                public Object doInDirContext(DirContext ctx)
-                    throws NamingException {
+        Boolean exists = (Boolean) executeReadOnly(new ContextExecutor() {
+                public Object executeWithContext(DirContext ctx) throws NamingException {
                     try {
                         Object obj = ctx.lookup(LdapUtils.getRelativeName(dn, ctx));
                         if (obj instanceof Context) {
@@ -161,12 +147,17 @@ public class LdapTemplate {
      *
      * @return the object created by the mapper
      */
-    public Object retrieveEntry(final String dn, final LdapEntryMapper mapper, final String[] attributesToRetrieve) {
-        return execute(new LdapCallback() {
-                public Object doInDirContext(DirContext ctx)
-                    throws NamingException {
-                    return mapper.mapAttributes(dn,
-                        ctx.getAttributes(LdapUtils.getRelativeName(dn, ctx), attributesToRetrieve));
+    public Object retrieveEntry(final String dn, final ContextMapper mapper, final String[] attributesToRetrieve) {
+
+        return executeReadOnly(new ContextExecutor() {
+                public Object executeWithContext(DirContext ctx) throws NamingException {
+                    Attributes attrs = ctx.getAttributes(LdapUtils.getRelativeName(dn, ctx), attributesToRetrieve);
+
+                    // Object object = ctx.lookup(LdapUtils.getRelativeName(dn, ctx));
+
+                    DirContextAdapter ctxAdapter = new DirContextAdapter(attrs, new DistinguishedName(dn));
+
+                    return mapper.mapFromContext(ctxAdapter);
                 }
             });
     }
@@ -185,8 +176,8 @@ public class LdapTemplate {
      */
     public Set searchForSingleAttributeValues(final String base, final String filter, final Object[] params,
         final String attributeName) {
-        class SingleAttributeSearchCallback implements LdapCallback {
-            public Object doInDirContext(DirContext ctx)
+        class SingleAttributeSearchCallback implements ContextExecutor {
+            public Object executeWithContext(DirContext ctx)
                 throws NamingException {
                 Set unionOfValues = new HashSet();
 
@@ -224,7 +215,7 @@ public class LdapTemplate {
             }
         }
 
-        return (Set) execute(new SingleAttributeSearchCallback());
+        return (Set) executeReadOnly(new SingleAttributeSearchCallback());
     }
 
     /**
@@ -242,10 +233,12 @@ public class LdapTemplate {
      *         result.
      */
     public Object searchForSingleEntry(final String base, final String filter, final Object[] params,
-        final LdapEntryMapper mapper) {
-        return execute(new LdapCallback() {
-                public Object doInDirContext(DirContext ctx)
+        final ContextMapper mapper) {
+
+        return executeReadOnly(new ContextExecutor() {
+                public Object executeWithContext(DirContext ctx)
                     throws NamingException {
+
                     NamingEnumeration results = ctx.search(base, filter, params, searchControls);
 
                     if (!results.hasMore()) {
@@ -274,7 +267,10 @@ public class LdapTemplate {
                         dn.append(nameInNamespace);
                     }
 
-                    return mapper.mapAttributes(dn.toString(), searchResult.getAttributes());
+                    DirContextAdapter ctxAdapter = new DirContextAdapter(
+                            searchResult.getAttributes(), new DistinguishedName(dn.toString()));
+
+                    return mapper.mapFromContext(ctxAdapter);
                 }
             });
     }

+ 58 - 10
core/src/main/java/org/acegisecurity/ldap/LdapUtils.java

@@ -19,6 +19,8 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
 import org.springframework.util.Assert;
+import org.springframework.ldap.support.DistinguishedName;
+import org.springframework.ldap.support.DirContextAdapter;
 
 import java.io.UnsupportedEncodingException;
 
@@ -45,6 +47,10 @@ public final class LdapUtils {
     //~ Methods ========================================================================================================
 
     public static void closeContext(Context ctx) {
+        if(ctx instanceof DirContextAdapter) {
+            return;
+        }
+
         try {
             if (ctx != null) {
                 ctx.close();
@@ -55,9 +61,10 @@ public final class LdapUtils {
     }
 
     /**
-     * Obtains the part of a DN relative to a supplied base context.<p>If the DN is
-     * "cn=bob,ou=people,dc=acegisecurity,dc=org" and the base context name is "ou=people,dc=acegisecurity,dc=org" it
-     * would return "cn=bob".</p>
+     * Obtains the part of a DN relative to a supplied base context.
+     * <p>If the DN is "cn=bob,ou=people,dc=acegisecurity,dc=org" and the base context name is
+     * "ou=people,dc=acegisecurity,dc=org" it would return "cn=bob".
+     * </p>
      *
      * @param fullDn the DN
      * @param baseCtx the context to work out the name relative to.
@@ -67,23 +74,43 @@ public final class LdapUtils {
      * @throws NamingException any exceptions thrown by the context are propagated.
      */
     public static String getRelativeName(String fullDn, Context baseCtx)
-        throws NamingException {
+            throws NamingException {
+
         String baseDn = baseCtx.getNameInNamespace();
 
         if (baseDn.length() == 0) {
             return fullDn;
         }
 
-        if (baseDn.equals(fullDn)) {
+        DistinguishedName base = new DistinguishedName(baseDn);
+        DistinguishedName full = new DistinguishedName(fullDn);
+
+        if(base.equals(full)) {
             return "";
         }
 
-        int index = fullDn.lastIndexOf(baseDn);
+        Assert.isTrue(full.startsWith(base), "Full DN does not start with base DN");
 
-        Assert.isTrue(index > 0, "Context base DN is not contained in the full DN");
+        full.removeFirst(base);
 
-        // remove the base name and preceding comma.
-        return fullDn.substring(0, index - 1);
+        return full.toString();
+    }
+
+    /**
+     * Gets the full dn of a name by prepending the name of the context it is relative to.
+     * If the name already contains the base name, it is returned unaltered.
+     */
+    public static DistinguishedName getFullDn(DistinguishedName dn, Context baseCtx)
+            throws NamingException {
+        DistinguishedName baseDn = new DistinguishedName(baseCtx.getNameInNamespace());
+
+        if(dn.contains(baseDn)) {
+            return dn;
+        }
+
+        baseDn.append(dn);
+
+        return baseDn;
     }
 
     public static byte[] getUtf8Bytes(String s) {
@@ -95,6 +122,27 @@ public final class LdapUtils {
         }
     }
 
+    public static String getUtf8BytesAsString(byte[] utf8) {
+        try {
+            return new String(utf8, "UTF-8");
+        } catch (UnsupportedEncodingException e) {
+            // Should be impossible since UTF-8 is required by all implementations
+            throw new IllegalStateException("Failed to convert string to UTF-8 bytes. Shouldn't be possible");
+        }
+    }
+
+    public static String convertPasswordToString(Object passObj) {
+        Assert.notNull(passObj, "Password object to convert must not be null");
+
+        if(passObj instanceof byte[]) {
+            return getUtf8BytesAsString((byte[])passObj);
+        } else if (passObj instanceof String) {
+            return (String)passObj;
+        } else {
+            throw new IllegalArgumentException("Password object was not a String or byte array.");
+        }
+    }
+
     /**
      * Works out the root DN for an LDAP URL.<p>For example, the URL
      * <tt>ldap://monkeymachine:11389/dc=acegisecurity,dc=org</tt> has the root DN "dc=acegisecurity,dc=org".</p>
@@ -136,7 +184,7 @@ public final class LdapUtils {
     }
 
     // removed for 1.3 compatibility
-/**
+    /**
      * 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

+ 9 - 4
core/src/main/java/org/acegisecurity/ldap/search/FilterBasedLdapUserSearch.java

@@ -30,6 +30,7 @@ import org.apache.commons.logging.LogFactory;
 import org.springframework.dao.IncorrectResultSizeDataAccessException;
 
 import org.springframework.util.Assert;
+import org.springframework.ldap.ContextSource;
 
 import javax.naming.directory.DirContext;
 import javax.naming.directory.SearchControls;
@@ -51,7 +52,7 @@ public class FilterBasedLdapUserSearch implements LdapUserSearch {
 
     //~ Instance fields ================================================================================================
 
-    private InitialDirContextFactory initialDirContextFactory;
+    private ContextSource initialDirContextFactory;
     private LdapUserDetailsMapper userDetailsMapper = new LdapUserDetailsMapper();
 
     /**
@@ -72,7 +73,6 @@ public class FilterBasedLdapUserSearch implements LdapUserSearch {
      *  <ul>
      *      <li>(uid={0}) - this would search for a username match on the uid attribute.</li>
      *  </ul>
-     *  TODO: more examples.
      */
     private String searchFilter;
 
@@ -116,9 +116,14 @@ public class FilterBasedLdapUserSearch implements LdapUserSearch {
         template.setSearchControls(searchControls);
 
         try {
-            LdapUserDetailsImpl.Essence user = (LdapUserDetailsImpl.Essence) template.searchForSingleEntry(searchBase,
-                    searchFilter, new String[] {username}, userDetailsMapper);
+
+            LdapUserDetailsImpl.Essence user = (LdapUserDetailsImpl.Essence) template.searchForSingleEntry(
+                    searchBase, searchFilter, new String[] {username}, userDetailsMapper);
+
             user.setUsername(username);
+//            if (!username.equals(user.getUsername())) {
+//                logger.debug("Search returned user object with different username: " + user.getUsername());
+//            }
 
             return user.createUserDetails();
         } catch (IncorrectResultSizeDataAccessException notFound) {

+ 5 - 6
core/src/main/java/org/acegisecurity/providers/ldap/authenticator/AbstractLdapAuthenticator.java

@@ -18,7 +18,6 @@ package org.acegisecurity.providers.ldap.authenticator;
 import org.acegisecurity.AcegiMessageSource;
 
 import org.acegisecurity.ldap.InitialDirContextFactory;
-import org.acegisecurity.ldap.LdapEntryMapper;
 import org.acegisecurity.ldap.LdapUserSearch;
 
 import org.acegisecurity.providers.ldap.LdapAuthenticator;
@@ -32,6 +31,7 @@ import org.springframework.context.MessageSourceAware;
 import org.springframework.context.support.MessageSourceAccessor;
 
 import org.springframework.util.Assert;
+import org.springframework.ldap.ContextMapper;
 
 import java.text.MessageFormat;
 
@@ -72,15 +72,14 @@ public abstract class AbstractLdapAuthenticator implements LdapAuthenticator, In
 
     /**
      * Create an initialized instance to the {@link InitialDirContextFactory} provided.
-     * 
+     *
      * @param initialDirContextFactory
      */
     public AbstractLdapAuthenticator(InitialDirContextFactory initialDirContextFactory) {
         this.setInitialDirContextFactory(initialDirContextFactory);
     }
 
-    // ~ Methods
-    // ========================================================================================================
+    //~ Methods ========================================================================================================
 
     public void afterPropertiesSet() throws Exception {
         Assert.isTrue((userDnFormat != null) || (userSearch != null),
@@ -89,7 +88,7 @@ public abstract class AbstractLdapAuthenticator implements LdapAuthenticator, In
 
     /**
      * Set the {@link InitialDirContextFactory} and initialize this instance from its data.
-     * 
+     *
      * @param initialDirContextFactory
      */
     private void setInitialDirContextFactory(InitialDirContextFactory initialDirContextFactory) {
@@ -111,7 +110,7 @@ public abstract class AbstractLdapAuthenticator implements LdapAuthenticator, In
         return userAttributes;
     }
 
-    protected LdapEntryMapper getUserDetailsMapper() {
+    protected ContextMapper getUserDetailsMapper() {
         return userDetailsMapper;
     }
 

+ 27 - 1
core/src/main/java/org/acegisecurity/providers/ldap/authenticator/BindAuthenticator.java

@@ -25,7 +25,10 @@ import org.acegisecurity.userdetails.ldap.LdapUserDetailsImpl;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.springframework.ldap.ContextSource;
+import org.springframework.dao.DataAccessException;
 
+import javax.naming.directory.DirContext;
 import java.util.Iterator;
 
 
@@ -81,7 +84,8 @@ public class BindAuthenticator extends AbstractLdapAuthenticator {
     }
 
     private LdapUserDetails bindWithDn(String userDn, String username, String password) {
-        LdapTemplate template = new LdapTemplate(getInitialDirContextFactory(), userDn, password);
+        LdapTemplate template = new LdapTemplate(
+                new BindWithSpecificDnContextSource(getInitialDirContextFactory(), userDn, password));
 
         try {
             LdapUserDetailsImpl.Essence user = (LdapUserDetailsImpl.Essence) template.retrieveEntry(userDn,
@@ -108,4 +112,26 @@ public class BindAuthenticator extends AbstractLdapAuthenticator {
             logger.debug("Failed to bind as " + userDn + ": " + cause);
         }
     }
+
+    private class BindWithSpecificDnContextSource implements ContextSource {
+        private InitialDirContextFactory ctxFactory;
+        private String userDn;
+        private String password;
+
+
+        public BindWithSpecificDnContextSource(InitialDirContextFactory ctxFactory, String userDn, String password) {
+            this.ctxFactory = ctxFactory;
+            this.userDn = userDn;
+            this.password = password;
+        }
+
+        public DirContext getReadOnlyContext() throws DataAccessException {
+            return ctxFactory.newInitialDirContext(userDn, password);
+        }
+
+        public DirContext getReadWriteContext() throws DataAccessException {
+            return getReadOnlyContext();
+        }
+    }
+
 }

+ 3 - 1
core/src/test/java/org/acegisecurity/ldap/DefaultInitialDirContextFactoryTests.java

@@ -17,6 +17,8 @@ package org.acegisecurity.ldap;
 
 import org.acegisecurity.AcegiMessageSource;
 import org.acegisecurity.BadCredentialsException;
+import org.springframework.ldap.UncategorizedLdapException;
+import org.springframework.ldap.support.DirContextAdapter;
 
 import java.util.Hashtable;
 
@@ -112,7 +114,7 @@ public class DefaultInitialDirContextFactoryTests extends AbstractLdapServerTest
         try {
             idf.newInitialDirContext();
             fail("Connection succeeded unexpectedly");
-        } catch (LdapDataAccessException expected) {}
+        } catch (UncategorizedLdapException expected) {}
     }
 
     public void testEnvironment() {

+ 11 - 10
core/src/test/java/org/acegisecurity/ldap/LdapTemplateTests.java

@@ -15,6 +15,9 @@
 
 package org.acegisecurity.ldap;
 
+import org.springframework.ldap.ContextExecutor;
+import org.springframework.ldap.UncategorizedLdapException;
+
 import java.util.Set;
 
 import javax.naming.NamingException;
@@ -64,21 +67,19 @@ public class LdapTemplateTests extends AbstractLdapServerTestCase {
 
     public void testNamingExceptionIsTranslatedCorrectly() {
         try {
-            template.execute(new LdapCallback() {
-                public Object doInDirContext(DirContext dirContext)
-                        throws NamingException {
-                    throw new NamingException();
-                }
-            });
-            fail("Expected LdapDataAccessException on NamingException");
-        } catch (LdapDataAccessException expected) {
-        }
+            template.executeReadOnly(new ContextExecutor() {
+                    public Object executeWithContext(DirContext dirContext) throws NamingException {
+                        throw new NamingException();
+                    }
+                });
+            fail("Expected UncategorizedLdapException on NamingException");
+        } catch (UncategorizedLdapException expected) {}
     }
 
     public void testSearchForSingleAttributeValues() {
         String param = "uid=ben,ou=people,dc=acegisecurity,dc=org";
 
-        Set values = template.searchForSingleAttributeValues("ou=groups", "(member={0})", new String[]{param}, "ou");
+        Set values = template.searchForSingleAttributeValues("ou=groups", "(member={0})", new String[] {param}, "ou");
 
         assertEquals("Expected 3 results from search", 3, values.size());
         assertTrue(values.contains("developer"));

+ 10 - 3
core/src/test/java/org/acegisecurity/ldap/LdapUtilsTests.java

@@ -30,9 +30,6 @@ import javax.naming.directory.DirContext;
  * @version $Id$
  */
 public class LdapUtilsTests extends MockObjectTestCase {
-    //~ Instance fields ================================================================================================
-
-    private final LdapDataAccessException tempCoverageBoost = new LdapDataAccessException("");
 
     //~ Methods ========================================================================================================
 
@@ -63,6 +60,16 @@ public class LdapUtilsTests extends MockObjectTestCase {
             LdapUtils.getRelativeName("cn=jane,dc=acegisecurity,dc=org", (Context) mockCtx.proxy()));
     }
 
+    public void testGetRelativeNameWorksWithArbitrarySpaces()
+        throws Exception {
+        Mock mockCtx = mock(DirContext.class);
+
+        mockCtx.expects(atLeastOnce()).method("getNameInNamespace").will(returnValue("dc=acegisecurity,dc = org"));
+
+        assertEquals("cn=jane smith",
+            LdapUtils.getRelativeName("cn=jane smith, dc = acegisecurity , dc=org", (Context) mockCtx.proxy()));
+    }
+
     public void testRootDnsAreParsedFromUrlsCorrectly() {
         assertEquals("", LdapUtils.parseRootDnFromUrl("ldap://monkeymachine"));
         assertEquals("", LdapUtils.parseRootDnFromUrl("ldap://monkeymachine/"));

+ 12 - 4
core/src/test/java/org/acegisecurity/ldap/MockInitialDirContextFactory.java

@@ -15,12 +15,12 @@
 
 package org.acegisecurity.ldap;
 
+import org.springframework.dao.DataAccessException;
+
 import javax.naming.directory.DirContext;
 
 
 /**
- * 
-DOCUMENT ME!
  *
  * @author Luke Taylor
  * @version $Id$
@@ -28,8 +28,8 @@ DOCUMENT ME!
 public class MockInitialDirContextFactory implements InitialDirContextFactory {
     //~ Instance fields ================================================================================================
 
-    DirContext ctx;
-    String baseDn;
+    private DirContext ctx;
+    private String baseDn;
 
     //~ Constructors ===================================================================================================
 
@@ -51,4 +51,12 @@ public class MockInitialDirContextFactory implements InitialDirContextFactory {
     public DirContext newInitialDirContext(String username, String password) {
         return ctx;
     }
+
+    public DirContext getReadOnlyContext() throws DataAccessException {
+        return ctx;
+    }
+
+    public DirContext getReadWriteContext() throws DataAccessException {
+        return ctx;
+    }
 }

+ 2 - 0
core/src/test/java/org/acegisecurity/ldap/search/FilterBasedLdapUserSearchTests.java

@@ -106,4 +106,6 @@ public class FilterBasedLdapUserSearchTests extends AbstractLdapServerTestCase {
 
 //        assertEquals("uid=ben,ou=people,dc=acegisecurity,dc=org", ben.getDn());
     }
+
+    // TODO: Add test with non-uid username
 }

+ 6 - 7
core/src/test/java/org/acegisecurity/providers/ldap/authenticator/PasswordComparisonAuthenticatorMockTests.java

@@ -26,8 +26,6 @@ import javax.naming.directory.DirContext;
 
 
 /**
- * 
-DOCUMENT ME!
  *
  * @author Luke Taylor
  * @version $Id$
@@ -46,15 +44,16 @@ public class PasswordComparisonAuthenticatorMockTests extends MockObjectTestCase
 
         // Get the mock to return an empty attribute set
         mockCtx.expects(atLeastOnce()).method("getNameInNamespace").will(returnValue("dc=acegisecurity,dc=org"));
-        mockCtx.expects(once()).method("lookup").with(eq("cn=Bob,ou=people")).will(returnValue(true));
-        mockCtx.expects(once()).method("getAttributes").with(eq("cn=Bob,ou=people"), NULL)
+        mockCtx.expects(once()).method("lookup").with(eq("cn=Bob, ou=people")).will(returnValue(true));
+        mockCtx.expects(once()).method("getAttributes").with(eq("cn=Bob, ou=people"), NULL)
                .will(returnValue(new BasicAttributes()));
 
         // Setup a single return value (i.e. success)
         Attributes searchResults = new BasicAttributes("", null);
-        mockCtx.expects(once()).method("search")
-               .with(eq("cn=Bob,ou=people"), eq("(userPassword={0})"), NOT_NULL, NOT_NULL)
-               .will(returnValue(searchResults.getAll()));
+        mockCtx.expects(once())
+                .method("search")
+                .with(eq("cn=Bob, ou=people"), eq("(userPassword={0})"), NOT_NULL, NOT_NULL)
+                .will(returnValue(searchResults.getAll()));
         mockCtx.expects(atLeastOnce()).method("close");
         authenticator.authenticate("Bob", "bobspassword");
     }