diff --git a/core/src/main/java/org/springframework/security/ldap/DefaultLdapUsernameToDnMapper.java b/core/src/main/java/org/springframework/security/ldap/DefaultLdapUsernameToDnMapper.java index dfba6c7d9a..eebaa043f1 100644 --- a/core/src/main/java/org/springframework/security/ldap/DefaultLdapUsernameToDnMapper.java +++ b/core/src/main/java/org/springframework/security/ldap/DefaultLdapUsernameToDnMapper.java @@ -3,6 +3,11 @@ package org.springframework.security.ldap; import org.springframework.ldap.core.DistinguishedName; /** + * This implementation appends a name component to the userDnBase context using the + * usernameAttributeName property. So if the uid attribute is used to store the username, and the + * base DN is cn=users and we are creating a new user called "sam", then the DN will be + * uid=sam,cn=users. + * * @author Luke Taylor * @version $Id$ */ @@ -11,11 +16,6 @@ public class DefaultLdapUsernameToDnMapper implements LdapUsernameToDnMapper { private String usernameAttribute; /** - * This implementation appends a name component to the userDnBase context using the - * usernameAttributeName property. So if the uid attribute is used to store the username, and the - * base DN is cn=users and we are creating a new user called "sam", then the DN will be - * uid=sam,cn=users. - * * @param userDnBase the base name of the DN * @param usernameAttribute the attribute to append for the username component. */ @@ -24,6 +24,9 @@ public class DefaultLdapUsernameToDnMapper implements LdapUsernameToDnMapper { this.usernameAttribute = usernameAttribute; } + /** + * Assembles the Distinguished Name that should be used the given username. + */ public DistinguishedName buildDn(String username) { DistinguishedName dn = new DistinguishedName(userDnBase); diff --git a/core/src/main/java/org/springframework/security/userdetails/ldap/LdapUserDetailsManager.java b/core/src/main/java/org/springframework/security/userdetails/ldap/LdapUserDetailsManager.java index 003db6aba0..669b9f72fa 100644 --- a/core/src/main/java/org/springframework/security/userdetails/ldap/LdapUserDetailsManager.java +++ b/core/src/main/java/org/springframework/security/userdetails/ldap/LdapUserDetailsManager.java @@ -119,7 +119,7 @@ public class LdapUserDetailsManager implements UserDetailsManager { } }; - private String[] attributesToRetrieve = null; + private String[] attributesToRetrieve; public LdapUserDetailsManager(ContextSource contextSource) { template = new LdapTemplate(contextSource); @@ -186,6 +186,7 @@ public class LdapUserDetailsManager implements UserDetailsManager { ctx.removeFromEnvironment("com.sun.jndi.ldap.connect.pool"); ctx.addToEnvironment(Context.SECURITY_PRINCIPAL, LdapUtils.getFullDn(dn, ctx).toUrl()); ctx.addToEnvironment(Context.SECURITY_CREDENTIALS, oldPassword); + // TODO: reconnect doesn't appear to actually change the credentials try { ctx.reconnect(null); } catch (javax.naming.AuthenticationException e) { @@ -317,11 +318,11 @@ public class LdapUserDetailsManager implements UserDetailsManager { userDetailsMapper.mapUserToContext(user, ctx); } - private void addAuthorities(DistinguishedName userDn, GrantedAuthority[] authorities) { + protected void addAuthorities(DistinguishedName userDn, GrantedAuthority[] authorities) { modifyAuthorities(userDn, authorities, DirContext.ADD_ATTRIBUTE); } - private void removeAuthorities(DistinguishedName userDn, GrantedAuthority[] authorities) { + protected void removeAuthorities(DistinguishedName userDn, GrantedAuthority[] authorities) { modifyAuthorities(userDn, authorities, DirContext.REMOVE_ATTRIBUTE); } diff --git a/core/src/test/java/org/springframework/security/userdetails/ldap/LdapUserDetailsManagerTests.java b/core/src/test/java/org/springframework/security/userdetails/ldap/LdapUserDetailsManagerTests.java index 02431df257..ed6b635e79 100644 --- a/core/src/test/java/org/springframework/security/userdetails/ldap/LdapUserDetailsManagerTests.java +++ b/core/src/test/java/org/springframework/security/userdetails/ldap/LdapUserDetailsManagerTests.java @@ -22,7 +22,6 @@ import org.springframework.security.ldap.AbstractLdapIntegrationTests; import org.springframework.security.ldap.DefaultLdapUsernameToDnMapper; import org.springframework.security.ldap.SpringSecurityLdapTemplate; import org.springframework.security.providers.UsernamePasswordAuthenticationToken; -import org.springframework.security.userdetails.UserDetails; import org.springframework.security.userdetails.UsernameNotFoundException; import org.springframework.ldap.core.DirContextAdapter; @@ -90,23 +89,17 @@ public class LdapUserDetailsManagerTests extends AbstractLdapIntegrationTests { public void testLoadUserByUsernameReturnsCorrectData() { mgr.setUsernameMapper(new DefaultLdapUsernameToDnMapper("ou=people","uid")); mgr.setGroupSearchBase("ou=groups"); - UserDetails bob = mgr.loadUserByUsername("bob"); + LdapUserDetails bob = (LdapUserDetails) mgr.loadUserByUsername("bob"); assertEquals("bob", bob.getUsername()); - // password isn't read - //assertEquals("bobspassword", bob.getPassword()); + assertEquals("uid=bob, ou=people, dc=springframework, dc=org", bob.getDn()); + assertEquals("bobspassword", bob.getPassword()); assertEquals(1, bob.getAuthorities().length); } - @Test + @Test(expected = UsernameNotFoundException.class) public void testLoadingInvalidUsernameThrowsUsernameNotFoundException() { - - try { - mgr.loadUserByUsername("jim"); - fail("Expected UsernameNotFoundException for user 'jim'"); - } catch(UsernameNotFoundException expected) { - // expected - } + mgr.loadUserByUsername("jim"); } @Test @@ -123,6 +116,7 @@ public class LdapUserDetailsManagerTests extends AbstractLdapIntegrationTests { @Test public void testCreateNewUserSucceeds() { InetOrgPerson.Essence p = new InetOrgPerson.Essence(); + p.setDn("whocares"); p.setCn(new String[] {"Joe Smeth"}); p.setSn("Smeth"); p.setUid("joe"); @@ -134,6 +128,7 @@ public class LdapUserDetailsManagerTests extends AbstractLdapIntegrationTests { @Test public void testDeleteUserSucceeds() { InetOrgPerson.Essence p = new InetOrgPerson.Essence(); + p.setDn("whocares"); p.setCn(new String[] {"Don Smeth"}); p.setSn("Smeth"); p.setUid("don"); @@ -162,6 +157,7 @@ public class LdapUserDetailsManagerTests extends AbstractLdapIntegrationTests { @Test public void testPasswordChangeWithCorrectOldPasswordSucceeds() { InetOrgPerson.Essence p = new InetOrgPerson.Essence(); + p.setDn("whocares"); p.setCn(new String[] {"John Yossarian"}); p.setSn("Yossarian"); p.setUid("johnyossarian"); @@ -179,9 +175,10 @@ public class LdapUserDetailsManagerTests extends AbstractLdapIntegrationTests { "userPassword", "yossariansnewpassword")); } - @Test + @Test(expected = BadCredentialsException.class) public void testPasswordChangeWithWrongOldPasswordFails() { InetOrgPerson.Essence p = new InetOrgPerson.Essence(); + p.setDn("whocares"); p.setCn(new String[] {"John Yossarian"}); p.setSn("Yossarian"); p.setUid("johnyossarian"); @@ -193,11 +190,6 @@ public class LdapUserDetailsManagerTests extends AbstractLdapIntegrationTests { SecurityContextHolder.getContext().setAuthentication( new UsernamePasswordAuthenticationToken("johnyossarian", "yossarianspassword", TEST_AUTHORITIES)); - try { - mgr.changePassword("wrongpassword", "yossariansnewpassword"); - fail("Expected BadCredentialsException"); - } catch (BadCredentialsException expected) { - } - + mgr.changePassword("wrongpassword", "yossariansnewpassword"); } }