Enforce the setting of a LdapUserDetailsMapper on authenticators (rather than a general mapper) to make sure the correct type is returned and that the username is set before it is returned.

This commit is contained in:
Luke Taylor 2006-05-22 23:40:29 +00:00
parent 3eaed3ad44
commit 4d24c88d1e
9 changed files with 46 additions and 21 deletions

View File

@ -18,10 +18,10 @@ package org.acegisecurity.ldap.search;
import org.acegisecurity.userdetails.UsernameNotFoundException;
import org.acegisecurity.userdetails.ldap.LdapUserDetailsMapper;
import org.acegisecurity.userdetails.ldap.LdapUserDetails;
import org.acegisecurity.userdetails.ldap.LdapUserDetailsImpl;
import org.acegisecurity.ldap.LdapUserSearch;
import org.acegisecurity.ldap.InitialDirContextFactory;
import org.acegisecurity.ldap.LdapTemplate;
import org.acegisecurity.ldap.LdapEntryMapper;
import org.springframework.util.Assert;
@ -80,7 +80,7 @@ public class FilterBasedLdapUserSearch implements LdapUserSearch {
private InitialDirContextFactory initialDirContextFactory;
private LdapEntryMapper userDetailsMapper = new LdapUserDetailsMapper();
private LdapUserDetailsMapper userDetailsMapper = new LdapUserDetailsMapper();
//~ Methods ================================================================
@ -122,10 +122,11 @@ public class FilterBasedLdapUserSearch implements LdapUserSearch {
template.setSearchControls(searchControls);
try {
Object user = template.searchForSingleEntry(searchBase, searchFilter, new String[] { username }, userDetailsMapper);
Assert.isInstanceOf(LdapUserDetails.class, user, "Entry mapper must return an LdapUserDetailsImpl instance");
LdapUserDetailsImpl.Essence user = (LdapUserDetailsImpl.Essence)
template.searchForSingleEntry(searchBase, searchFilter, new String[] { username }, userDetailsMapper);
user.setUsername(username);
return (LdapUserDetails)user;
return user.createUserDetails();
} catch(EmptyResultDataAccessException notFound) {
throw new UsernameNotFoundException("User " + username + " not found in directory.");

View File

@ -57,7 +57,7 @@ public abstract class AbstractLdapAuthenticator implements LdapAuthenticator,
/** The attributes which will be retrieved from the directory. Null means all attributes */
private String[] userAttributes = null;
private LdapEntryMapper userDetailsMapper = new LdapUserDetailsMapper();
private LdapUserDetailsMapper userDetailsMapper = new LdapUserDetailsMapper();
/**
* The suffix to be added to the DN patterns, worked out internally from the root DN of the
@ -141,7 +141,7 @@ public abstract class AbstractLdapAuthenticator implements LdapAuthenticator,
this.userSearch = userSearch;
}
public void setUserDetailsMapper(LdapEntryMapper userDetailsMapper) {
public void setUserDetailsMapper(LdapUserDetailsMapper userDetailsMapper) {
Assert.notNull("userDetailsMapper must not be null");
this.userDetailsMapper = userDetailsMapper;
}

View File

@ -19,10 +19,10 @@ import org.acegisecurity.ldap.InitialDirContextFactory;
import org.acegisecurity.ldap.LdapTemplate;
import org.acegisecurity.BadCredentialsException;
import org.acegisecurity.userdetails.ldap.LdapUserDetails;
import org.acegisecurity.userdetails.ldap.LdapUserDetailsImpl;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.util.Assert;
import java.util.Iterator;
@ -57,14 +57,14 @@ public class BindAuthenticator extends AbstractLdapAuthenticator {
Iterator dns = getUserDns(username).iterator();
while(dns.hasNext() && user == null) {
user = bindWithDn((String)dns.next(), password);
user = bindWithDn((String)dns.next(), username, password);
}
// Otherwise use the configured locator to find the user
// and authenticate with the returned DN.
if (user == null && getUserSearch() != null) {
LdapUserDetails userFromSearch = getUserSearch().searchForUser(username);
user = bindWithDn(userFromSearch.getDn(), password);
user = bindWithDn(userFromSearch.getDn(), username, password);
}
if(user == null) {
@ -77,7 +77,7 @@ public class BindAuthenticator extends AbstractLdapAuthenticator {
}
LdapUserDetails bindWithDn(String userDn, String password) {
private LdapUserDetails bindWithDn(String userDn, String username, String password) {
LdapTemplate template = new LdapTemplate(getInitialDirContextFactory(), userDn, password);
if (logger.isDebugEnabled()) {
@ -86,10 +86,11 @@ public class BindAuthenticator extends AbstractLdapAuthenticator {
try {
Object user = (LdapUserDetails)template.retrieveEntry(userDn, getUserDetailsMapper(), getUserAttributes());
Assert.isInstanceOf(LdapUserDetails.class, user, "Entry mapper must return an LdapUserDetails instance");
LdapUserDetailsImpl.Essence user =
(LdapUserDetailsImpl.Essence) template.retrieveEntry(userDn, getUserDetailsMapper(), getUserAttributes());
user.setUsername(username);
return (LdapUserDetails) user;
return user.createUserDetails();
} catch(BadCredentialsException e) {
// This will be thrown if an invalid user name is used and the method may

View File

@ -19,6 +19,7 @@ import org.acegisecurity.ldap.LdapUtils;
import org.acegisecurity.ldap.InitialDirContextFactory;
import org.acegisecurity.ldap.LdapTemplate;
import org.acegisecurity.userdetails.ldap.LdapUserDetails;
import org.acegisecurity.userdetails.ldap.LdapUserDetailsImpl;
import org.acegisecurity.providers.encoding.PasswordEncoder;
import org.acegisecurity.BadCredentialsException;
import org.acegisecurity.userdetails.UsernameNotFoundException;
@ -81,7 +82,10 @@ public final class PasswordComparisonAuthenticator extends AbstractLdapAuthentic
final String userDn = (String)dns.next();
if(ldapTemplate.nameExists(userDn)) {
user = (LdapUserDetails)ldapTemplate.retrieveEntry(userDn, getUserDetailsMapper(), getUserAttributes());
LdapUserDetailsImpl.Essence userEssence =
(LdapUserDetailsImpl.Essence) ldapTemplate.retrieveEntry(userDn, getUserDetailsMapper(), getUserAttributes());
userEssence.setUsername(username);
user = userEssence.createUserDetails();
}
}

View File

@ -1,6 +1,7 @@
package org.acegisecurity.userdetails.ldap;
import org.acegisecurity.GrantedAuthority;
import org.springframework.util.Assert;
import javax.naming.directory.Attributes;
import javax.naming.directory.BasicAttributes;
@ -93,7 +94,9 @@ public class LdapUserDetailsImpl implements LdapUserDetails {
//~ Inner classes ==========================================================
/** Variation of essence pattern. Used to create mutable intermediate object */
/**
* Variation of essence pattern. Used to create mutable intermediate object
*/
public static class Essence {
LdapUserDetailsImpl instance = new LdapUserDetailsImpl();
@ -178,9 +181,15 @@ public class LdapUserDetailsImpl implements LdapUserDetails {
public LdapUserDetails createUserDetails() {
//TODO: Validation of properties
Assert.notNull(instance, "Essence can only be used to create a single instance");
instance.authorities = getGrantedAuthorities();
return instance;
LdapUserDetails newInstance = instance;
instance = null;
return newInstance;
}
}
}

View File

@ -27,6 +27,10 @@ import javax.naming.NamingException;
import javax.naming.NamingEnumeration;
/**
* The entry mapper used by the authenticators to create an ldap user
* object.
*
*
* @author Luke Taylor
* @version $Id$
*/
@ -100,6 +104,6 @@ public class LdapUserDetailsMapper implements LdapEntryMapper {
}
}
return essence.createUserDetails();
return essence;
}
}

View File

@ -37,6 +37,7 @@ public class FilterBasedLdapUserSearchTests extends AbstractLdapServerTestCase {
locator.setDerefLinkFlag(false);
LdapUserDetails bob = locator.searchForUser("bob");
assertEquals("bob", bob.getUsername());
// name is wrong with embedded apacheDS
// assertEquals("uid=bob,ou=people,dc=acegisecurity,dc=org", bob.getDn());
}
@ -48,6 +49,7 @@ public class FilterBasedLdapUserSearchTests extends AbstractLdapServerTestCase {
locator.setSearchSubtree(true);
LdapUserDetails ben = locator.searchForUser("Ben Alex");
assertEquals("Ben Alex", ben.getUsername());
// assertEquals("uid=ben,ou=people,dc=acegisecurity,dc=org", ben.getDn());
}

View File

@ -32,6 +32,7 @@ public class BindAuthenticatorTests extends AbstractLdapServerTestCase {
public void testAuthenticationWithCorrectPasswordSucceeds() {
authenticator.setUserDnPatterns(new String[] {"uid={0},ou=people"});
LdapUserDetails user = authenticator.authenticate("bob","bobspassword");
assertEquals("bob", user.getUsername());
}
public void testAuthenticationWithWrongPasswordFails() {

View File

@ -65,7 +65,10 @@ public class PasswordComparisonAuthenticatorTests extends AbstractLdapServerTest
}
*/
public void testLocalPasswordComparisonSucceedsWithCorrectPassword() {
authenticator.authenticate("Bob", "bobspassword");
LdapUserDetails user = authenticator.authenticate("Bob", "bobspassword");
// check username is retrieved.
assertEquals("Bob", user.getUsername());
assertEquals("bobspassword", user.getPassword());
}
public void testMultipleDnPatternsWorkOk() {
@ -88,7 +91,7 @@ public class PasswordComparisonAuthenticatorTests extends AbstractLdapServerTest
public void testAllAttributesAreRetrivedByDefault() {
LdapUserDetails user = authenticator.authenticate("Bob", "bobspassword");
System.out.println(user.getAttributes().toString());
//System.out.println(user.getAttributes().toString());
assertEquals("User should have 5 attributes", 5, user.getAttributes().size());
}
@ -107,7 +110,7 @@ public class PasswordComparisonAuthenticatorTests extends AbstractLdapServerTest
mapper.setPasswordAttributeName("uid");
authenticator.setPasswordAttributeName("uid");
authenticator.setUserDetailsMapper(mapper);
authenticator.authenticate("bob", "bob");
LdapUserDetails bob = authenticator.authenticate("bob", "bob");
}
/*
public void testLdapCompareWithDifferentPasswordAttributeSucceeds() {