Make sure populator roles are added rather than overwriting any roles loaded with the user entry.

core/src/main/java/org/acegisecurity/providers/ldap/LdapAuthenticationProvider.java

@@ -22,6 +22,7 @@ import org.acegisecurity.userdetails.ldap.LdapUserDetailsImpl;
 import org.acegisecurity.userdetails.ldap.LdapUserDetails;
 import org.acegisecurity.AuthenticationException;
 import org.acegisecurity.BadCredentialsException;
+import org.acegisecurity.GrantedAuthority;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -129,10 +130,6 @@ public class LdapAuthenticationProvider extends AbstractUserDetailsAuthenticatio
         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 ================================================================
@@ -180,7 +177,11 @@ public class LdapAuthenticationProvider extends AbstractUserDetailsAuthenticatio
 
         LdapUserDetailsImpl.Essence user = new LdapUserDetailsImpl.Essence(ldapUser);
 
-        user.setAuthorities(authoritiesPopulator.getGrantedAuthorities(ldapUser));
+        GrantedAuthority[] extraAuthorities = authoritiesPopulator.getGrantedAuthorities(ldapUser);
+
+        for(int i = 0; i < extraAuthorities.length; i++) {
+            user.addAuthority(extraAuthorities[i]);
+        }
 
         return user.createUserDetails();
     }

core/src/test/java/org/acegisecurity/providers/ldap/LdapAuthenticationProviderTests.java

@@ -7,19 +7,18 @@ import org.acegisecurity.GrantedAuthority;
 import org.acegisecurity.GrantedAuthorityImpl;
 import org.acegisecurity.BadCredentialsException;
 import org.acegisecurity.ldap.*;
-import org.acegisecurity.ldap.DefaultInitialDirContextFactory;
 import org.acegisecurity.providers.UsernamePasswordAuthenticationToken;
 import org.acegisecurity.userdetails.UserDetails;
 import org.acegisecurity.userdetails.ldap.LdapUserDetailsImpl;
 import org.acegisecurity.userdetails.ldap.LdapUserDetails;
 
+import java.util.ArrayList;
+
 /**
  * @author Luke Taylor
  * @version $Id$
  */
 public class LdapAuthenticationProviderTests extends AbstractLdapServerTestCase {
-    DefaultInitialDirContextFactory dirCtxFactory;
-
 
     public LdapAuthenticationProviderTests(String string) {
         super(string);
@@ -29,62 +28,49 @@ public class LdapAuthenticationProviderTests extends AbstractLdapServerTestCase
         super();
     }
 
-    public void testNormalUsage() throws Exception {
+    public void testNormalUsage() {
         LdapAuthenticationProvider ldapProvider
                 = new LdapAuthenticationProvider(new MockAuthenticator(), new MockAuthoritiesPopulator());
 
         UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("bob","bobspassword");
         UserDetails user = ldapProvider.retrieveUser("bob", token);
-        assertEquals(1, user.getAuthorities().length);
-        assertTrue(user.getAuthorities()[0].equals("ROLE_USER"));
-        ldapProvider.additionalAuthenticationChecks(user, token);
+        assertEquals(2, user.getAuthorities().length);
+
+        ArrayList authorities = new ArrayList();
+        authorities.add(user.getAuthorities()[0].getAuthority());
+        authorities.add(user.getAuthorities()[1].getAuthority());
+
+        assertTrue(authorities.contains("ROLE_FROM_ENTRY"));
+        assertTrue(authorities.contains("ROLE_FROM_POPULATOR"));
 
+        ldapProvider.additionalAuthenticationChecks(user, token);
     }
-/*
+
 // This test kills apacheDS in embedded mode because the search returns an invalid DN
-    public void testIntegration() throws Exception {
-        LdapAuthenticationProvider ldapProvider = new LdapAuthenticationProvider();
-
-        // Connection information
-        DefaultInitialDirContextFactory dirCtxFactory = new DefaultInitialDirContextFactory();
-        dirCtxFactory.setUrl(PROVIDER_URL);
-        dirCtxFactory.setManagerDn(MANAGER_USER);
-        dirCtxFactory.setInitialContextFactory(CONTEXT_FACTORY);
-        dirCtxFactory.setExtraEnvVars(EXTRA_ENV);
-        dirCtxFactory.setManagerPassword(MANAGER_PASSWORD);
-        dirCtxFactory.afterPropertiesSet();
-        BindAuthenticator authenticator = new BindAuthenticator();
-        //PasswordComparisonAuthenticator authenticator = new PasswordComparisonAuthenticator();
-        authenticator.setInitialDirContextFactory(dirCtxFactory);
-        //authenticator.setUserDnPatterns("cn={0},ou=people");
-
-        FilterBasedLdapUserSearch userSearch = new FilterBasedLdapUserSearch();
-        userSearch.setSearchBase("ou=people");
-        userSearch.setSearchFilter("(cn={0})");
-        userSearch.setInitialDirContextFactory(dirCtxFactory);
-        userSearch.afterPropertiesSet();
-
-        authenticator.setUserSearch(userSearch);
-
-        authenticator.afterPropertiesSet();
-
-        DefaultLdapAuthoritiesPopulator populator;
-        populator = new DefaultLdapAuthoritiesPopulator();
-        populator.setRolePrefix("ROLE_");
-        populator.setInitialDirContextFactory(dirCtxFactory);
-        populator.setGroupSearchBase("ou=groups");
-        populator.afterPropertiesSet();
-
-        ldapProvider.setAuthoritiesPopulator(populator);
-        ldapProvider.setAuthenticator(authenticator);
-        Authentication auth = ldapProvider.authenticate(new UsernamePasswordAuthenticationToken("Ben Alex","benspassword"));
-        assertEquals(2, auth.getAuthorities().length);
-    }
-*/
+//    public void testIntegration() throws Exception {
+//        BindAuthenticator authenticator = new BindAuthenticator(getInitialCtxFactory());
+//        //PasswordComparisonAuthenticator authenticator = new PasswordComparisonAuthenticator();
+//        //authenticator.setUserDnPatterns("cn={0},ou=people");
+//
+//        FilterBasedLdapUserSearch userSearch = new FilterBasedLdapUserSearch("ou=people", "(cn={0})", getInitialCtxFactory());
+//
+//        authenticator.setUserSearch(userSearch);
+//        authenticator.afterPropertiesSet();
+//
+//        DefaultLdapAuthoritiesPopulator populator;
+//        populator = new DefaultLdapAuthoritiesPopulator(getInitialCtxFactory(), "ou=groups");
+//        populator.setRolePrefix("ROLE_");
+//
+//        LdapAuthenticationProvider ldapProvider = new LdapAuthenticationProvider(authenticator, populator);
+//
+//        Authentication auth = ldapProvider.authenticate(new UsernamePasswordAuthenticationToken("Ben Alex","benspassword"));
+//        assertEquals(2, auth.getAuthorities().length);
+//    }
+
     class MockAuthoritiesPopulator implements LdapAuthoritiesPopulator {
 
         public GrantedAuthority[] getGrantedAuthorities(LdapUserDetails userDetailsll) {
-           return new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_USER") };
+           return new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_FROM_POPULATOR") };
         }
     }
 
@@ -93,10 +79,11 @@ public class LdapAuthenticationProviderTests extends AbstractLdapServerTestCase
 
         public LdapUserDetails authenticate(String username, String password) {
             if(username.equals("bob") && password.equals("bobspassword")) {
-                LdapUserDetailsImpl.Essence creator = new LdapUserDetailsImpl.Essence();
-                creator.setDn("cn=bob,ou=people,dc=acegisecurity,dc=org");
-                creator.setAttributes(userAttributes);
-                return creator.createUserDetails();
+                LdapUserDetailsImpl.Essence userEssence = new LdapUserDetailsImpl.Essence();
+                userEssence.setDn("cn=bob,ou=people,dc=acegisecurity,dc=org");
+                userEssence.setAttributes(userAttributes);
+                userEssence.addAuthority(new GrantedAuthorityImpl("ROLE_FROM_ENTRY"));
+                return userEssence.createUserDetails();
             }
             throw new BadCredentialsException("Authentication of Bob failed.");
         }