From d3e42c6f3f24a800778ef4212f8a87b7908c9af6 Mon Sep 17 00:00:00 2001
From: Luke Taylor <luke.taylor@springsource.com>
Date: Fri, 19 May 2006 22:29:17 +0000
Subject: [PATCH] Move conversion of roles to Strings into LdapTemplate

---
 .../org/acegisecurity/ldap/LdapTemplate.java  | 20 ++++++++++++++-----
 .../DefaultLdapAuthoritiesPopulator.java      | 17 ++++++----------
 2 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/core/src/main/java/org/acegisecurity/ldap/LdapTemplate.java b/core/src/main/java/org/acegisecurity/ldap/LdapTemplate.java
index 28f3c09500..61174298d3 100644
--- a/core/src/main/java/org/acegisecurity/ldap/LdapTemplate.java
+++ b/core/src/main/java/org/acegisecurity/ldap/LdapTemplate.java
@@ -53,6 +53,8 @@ public class LdapTemplate {
     /** Default search controls */
     private SearchControls searchControls = new SearchControls();
 
+    private NamingExceptionTranslator exceptionTranslator = new LdapExceptionTranslator();
+
     public LdapTemplate(InitialDirContextFactory dirContextFactory) {
         Assert.notNull(dirContextFactory, "An InitialDirContextFactory is required");
         this.dirContextFactory = dirContextFactory;
@@ -112,8 +114,7 @@ public class LdapTemplate {
             return callback.execute(ctx);
 
         } catch (NamingException exception) {
-            // TODO: Write a static method in separate NamingExceptionExceptionTranslator class called public DataAccessException convert(NamingException);
-            throw new LdapDataAccessException("xxxx", exception);
+            throw exceptionTranslator.translate("LdapCallback", exception);
         } finally {
             LdapUtils.closeContext(ctx);
         }
@@ -148,13 +149,13 @@ public class LdapTemplate {
     /**
      * Performs a search using the supplied filter and returns the union of the values of the named
      * attribute found in all entries matched by the search. Note that one directory entry may have several
-     * values for the attribute.
+     * values for the attribute. Intended for role searches and similar scenarios.
      *
      * @param base the DN to search in
      * @param filter search filter to use
      * @param params the parameters to substitute in the search filter
      * @param attributeName the attribute who's values are to be retrieved.
-     * @return the set of values for the attribute as a union of the values found in all the matching entries.
+     * @return the set of String values for the attribute as a union of the values found in all the matching entries.
      */
     public Set searchForSingleAttributeValues(final String base, final String filter, final Object[] params, final String attributeName) {
 
@@ -187,7 +188,8 @@ public class LdapTemplate {
 
                         while(attributeValues.hasMore()) {
                             Object value = attributeValues.next();
-                            unionOfValues.add(value);
+
+                            unionOfValues.add(value.toString());
                         }
 
                     }
@@ -288,4 +290,12 @@ public class LdapTemplate {
         );
     }
 
+
+    private static class LdapExceptionTranslator implements NamingExceptionTranslator {
+
+        public DataAccessException translate(String task, NamingException e) {
+            return new LdapDataAccessException(task + ";" + e.getMessage(), e);
+        }
+    }
+
 }
diff --git a/core/src/main/java/org/acegisecurity/providers/ldap/populator/DefaultLdapAuthoritiesPopulator.java b/core/src/main/java/org/acegisecurity/providers/ldap/populator/DefaultLdapAuthoritiesPopulator.java
index 28bee4dde9..2f6f4c1f09 100644
--- a/core/src/main/java/org/acegisecurity/providers/ldap/populator/DefaultLdapAuthoritiesPopulator.java
+++ b/core/src/main/java/org/acegisecurity/providers/ldap/populator/DefaultLdapAuthoritiesPopulator.java
@@ -211,7 +211,7 @@ public class DefaultLdapAuthoritiesPopulator implements LdapAuthoritiesPopulator
         }
 
         if (logger.isDebugEnabled()) {
-            logger.debug("Searching for roles for user '" + username + "', DN = " + "'" 
+            logger.debug("Searching for roles for user '" + username + "', DN = " + "'"
                     + userDn + "', with filter "+ groupSearchFilter
                     + " in search base '" + groupSearchBase + "'");
         }
@@ -229,18 +229,13 @@ public class DefaultLdapAuthoritiesPopulator implements LdapAuthoritiesPopulator
         Iterator it = userRoles.iterator();
 
         while(it.hasNext()) {
-            Object role = it.next();
+            String role = (String) it.next();
 
-            // We only handle Strings for the time being
-            if(role instanceof String) {
-                if(convertToUpperCase) {
-                    role = ((String)role).toUpperCase();
-                }
-
-                authorities.add(new GrantedAuthorityImpl(rolePrefix + role));
-            } else {
-                logger.warn("Non-String value found for role: " + role);
+            if(convertToUpperCase) {
+                role = role.toUpperCase();
             }
+
+            authorities.add(new GrantedAuthorityImpl(rolePrefix + role));
         }
 
         return authorities;