From fc8ead3c540ad18a26b2dba5fcb9f2b5bffbe34c Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Tue, 16 May 2006 23:33:02 +0000 Subject: [PATCH] Make sure populator roles are added rather than overwriting any roles loaded with the user entry. --- .../ldap/LdapAuthenticationProvider.java | 11 +-- .../ldap/LdapAuthenticationProviderTests.java | 89 ++++++++----------- 2 files changed, 44 insertions(+), 56 deletions(-) diff --git a/core/src/main/java/org/acegisecurity/providers/ldap/LdapAuthenticationProvider.java b/core/src/main/java/org/acegisecurity/providers/ldap/LdapAuthenticationProvider.java index 5f8e66bcc8..a79a1a0d73 100644 --- a/core/src/main/java/org/acegisecurity/providers/ldap/LdapAuthenticationProvider.java +++ b/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(); } diff --git a/core/src/test/java/org/acegisecurity/providers/ldap/LdapAuthenticationProviderTests.java b/core/src/test/java/org/acegisecurity/providers/ldap/LdapAuthenticationProviderTests.java index 52d2f0df01..0189617291 100644 --- a/core/src/test/java/org/acegisecurity/providers/ldap/LdapAuthenticationProviderTests.java +++ b/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")); + 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(); +// 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); +// } - // 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); - } -*/ 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."); }