Explorar el Código

SEC-449: Make LdapUserDetailsMapper a pure ContextMapper so it can be used with LdapTemplate.

Luke Taylor hace 18 años
padre
commit
d208cf3824

+ 9 - 6
core/src/main/java/org/acegisecurity/ldap/search/FilterBasedLdapUserSearch.java

@@ -117,15 +117,14 @@ public class FilterBasedLdapUserSearch implements LdapUserSearch {
 
         try {
 
-            LdapUserDetailsImpl.Essence user = (LdapUserDetailsImpl.Essence) template.searchForSingleEntry(
+            LdapUserDetailsImpl user = (LdapUserDetailsImpl) template.searchForSingleEntry(
                     searchBase, searchFilter, new String[] {username}, userDetailsMapper);
 
-            user.setUsername(username);
-//            if (!username.equals(user.getUsername())) {
-//                logger.debug("Search returned user object with different username: " + user.getUsername());
-//            }
+            if (!username.equals(user.getUsername())) {
+                logger.debug("Search returned user object with different username: " + user.getUsername());
+            }
 
-            return user.createUserDetails();
+            return user;
         } catch (IncorrectResultSizeDataAccessException notFound) {
             if (notFound.getActualSize() == 0) {
                 throw new UsernameNotFoundException("User " + username + " not found in directory.");
@@ -164,6 +163,10 @@ public class FilterBasedLdapUserSearch implements LdapUserSearch {
         searchControls.setTimeLimit(searchTimeLimit);
     }
 
+    public void setUserDetailsMapper(LdapUserDetailsMapper userDetailsMapper) {
+        this.userDetailsMapper = userDetailsMapper;
+    }
+
     public String toString() {
         StringBuffer sb = new StringBuffer();
 

+ 2 - 3
core/src/main/java/org/acegisecurity/providers/ldap/authenticator/BindAuthenticator.java

@@ -88,11 +88,10 @@ public class BindAuthenticator extends AbstractLdapAuthenticator {
                 new BindWithSpecificDnContextSource(getInitialDirContextFactory(), userDn, password));
 
         try {
-            LdapUserDetailsImpl.Essence user = (LdapUserDetailsImpl.Essence) template.retrieveEntry(userDn,
+            LdapUserDetailsImpl user = (LdapUserDetailsImpl) template.retrieveEntry(userDn,
                     getUserDetailsMapper(), getUserAttributes());
-            user.setUsername(username);
 
-            return user.createUserDetails();
+            return user;
         } catch (BadCredentialsException e) {
             // This will be thrown if an invalid user name is used and the method may
             // be called multiple times to try different names, so we trap the exception

+ 2 - 4
core/src/main/java/org/acegisecurity/providers/ldap/authenticator/PasswordComparisonAuthenticator.java

@@ -82,14 +82,12 @@ public final class PasswordComparisonAuthenticator extends AbstractLdapAuthentic
             final String userDn = (String) dns.next();
 
             if (ldapTemplate.nameExists(userDn)) {
-                LdapUserDetailsImpl.Essence userEssence = (LdapUserDetailsImpl.Essence)
+                user = (LdapUserDetailsImpl)
                         ldapTemplate.retrieveEntry(userDn, getUserDetailsMapper(), getUserAttributes());
-                userEssence.setUsername(username);
-                user = userEssence.createUserDetails();
             }
         }
 
-        if ((user == null) && (getUserSearch() != null)) {
+        if (user == null && getUserSearch() != null) {
             user = getUserSearch().searchForUser(username);
         }
 

+ 24 - 24
core/src/main/java/org/acegisecurity/userdetails/ldap/LdapUserDetailsMapper.java

@@ -40,7 +40,7 @@ public class LdapUserDetailsMapper implements ContextMapper {
     //~ Instance fields ================================================================================================
 
     private final Log logger = LogFactory.getLog(LdapUserDetailsMapper.class);
-//    private String usernameAttributeName = "uid";
+    private String usernameAttributeName = "uid";
     private String passwordAttributeName = "userPassword";
     private String rolePrefix = "ROLE_";
     private String[] roleAttributes = null;
@@ -66,7 +66,7 @@ public class LdapUserDetailsMapper implements ContextMapper {
             essence.setPassword(mapPassword(passwordAttribute));
         }
 
-//        essence.setUsername(mapUsername(ctx));
+        essence.setUsername(mapUsername(ctx));
 
         // Map the roles
         for (int i = 0; (roleAttributes != null) && (i < roleAttributes.length); i++) {
@@ -86,8 +86,8 @@ public class LdapUserDetailsMapper implements ContextMapper {
             }
         }
 
-        //return essence.createUserDetails();
-        return essence;
+        return essence.createUserDetails();
+        //return essence;
     }
 
     /**
@@ -115,23 +115,23 @@ public class LdapUserDetailsMapper implements ContextMapper {
 
     }
 
-//    protected String mapUsername(DirContextAdapter ctx) {
-//        Attribute usernameAttribute = ctx.getAttributes().get(usernameAttributeName);
-//        String username;
-//
-//        if (usernameAttribute == null) {
-//            throw new AttributesIntegrityViolationException(
-//                    "Failed to get attribute " + usernameAttributeName + " from context");
-//        }
-//
-//        try {
-//            username = (String) usernameAttribute.get();
-//        } catch (NamingException e) {
-//            throw new UncategorizedLdapException("Failed to get username from attribute " + usernameAttributeName, e);
-//        }
-//
-//        return username;
-//    }
+    protected String mapUsername(DirContextAdapter ctx) {
+        Attribute usernameAttribute = ctx.getAttributes().get(usernameAttributeName);
+        String username;
+
+        if (usernameAttribute == null) {
+            throw new UncategorizedLdapException(
+                    "Failed to get attribute " + usernameAttributeName + " from context");
+        }
+
+        try {
+            username = (String) usernameAttribute.get();
+        } catch (NamingException e) {
+            throw new UncategorizedLdapException("Failed to get username from attribute " + usernameAttributeName, e);
+        }
+
+        return username;
+    }
 
     /**
      * Creates a GrantedAuthority from a role attribute. Override to customize
@@ -176,9 +176,9 @@ public class LdapUserDetailsMapper implements ContextMapper {
     }
 
 
-//    public void setUsernameAttributeName(String usernameAttributeName) {
-//        this.usernameAttributeName = usernameAttributeName;
-//    }
+    public void setUsernameAttributeName(String usernameAttributeName) {
+        this.usernameAttributeName = usernameAttributeName;
+    }
 
     /**
      * The names of any attributes in the user's  entry which represent application

+ 1 - 1
core/src/test/java/org/acegisecurity/ldap/search/FilterBasedLdapUserSearchTests.java

@@ -92,7 +92,7 @@ public class FilterBasedLdapUserSearchTests extends AbstractLdapIntegrationTests
         locator.setSearchSubtree(true);
 
         LdapUserDetails ben = locator.searchForUser("Ben Alex");
-        assertEquals("Ben Alex", ben.getUsername());
+        assertEquals("ben", ben.getUsername());
 
 //        assertEquals("uid=ben,ou=people,dc=acegisecurity,dc=org", ben.getDn());
     }

+ 5 - 3
core/src/test/java/org/acegisecurity/providers/ldap/authenticator/PasswordComparisonAuthenticatorMockTests.java

@@ -23,6 +23,7 @@ import org.jmock.MockObjectTestCase;
 import javax.naming.directory.Attributes;
 import javax.naming.directory.BasicAttributes;
 import javax.naming.directory.DirContext;
+import javax.naming.directory.BasicAttribute;
 
 
 /**
@@ -33,9 +34,10 @@ import javax.naming.directory.DirContext;
 public class PasswordComparisonAuthenticatorMockTests extends MockObjectTestCase {
     //~ Methods ========================================================================================================
 
-    public void testLdapCompareIsUsedWhenPasswordIsNotRetrieved()
-        throws Exception {
+    public void testLdapCompareIsUsedWhenPasswordIsNotRetrieved() throws Exception {
         Mock mockCtx = mock(DirContext.class);
+        BasicAttributes attrs = new BasicAttributes();
+        attrs.put(new BasicAttribute("uid", "bob"));
 
         PasswordComparisonAuthenticator authenticator = new PasswordComparisonAuthenticator(new MockInitialDirContextFactory(
                     (DirContext) mockCtx.proxy(), "dc=acegisecurity,dc=org"));
@@ -46,7 +48,7 @@ public class PasswordComparisonAuthenticatorMockTests extends MockObjectTestCase
         mockCtx.expects(atLeastOnce()).method("getNameInNamespace").will(returnValue("dc=acegisecurity,dc=org"));
         mockCtx.expects(once()).method("lookup").with(eq("cn=Bob, ou=people")).will(returnValue(true));
         mockCtx.expects(once()).method("getAttributes").with(eq("cn=Bob, ou=people"), NULL)
-               .will(returnValue(new BasicAttributes()));
+               .will(returnValue(attrs));
 
         // Setup a single return value (i.e. success)
         Attributes searchResults = new BasicAttributes("", null);

+ 10 - 10
core/src/test/java/org/acegisecurity/providers/ldap/authenticator/PasswordComparisonAuthenticatorTests.java

@@ -86,7 +86,7 @@ public class PasswordComparisonAuthenticatorTests extends AbstractLdapIntegratio
 
    public void testLdapPasswordCompareFailsWithWrongPassword() {
        // Don't retrieve the password
-       authenticator.setUserAttributes(new String[] {"cn", "sn"});
+       authenticator.setUserAttributes(new String[] {"uid", "cn", "sn"});
        try {
            authenticator.authenticate("Bob", "wrongpassword");
            fail("Authentication should fail with wrong password.");
@@ -95,9 +95,9 @@ public class PasswordComparisonAuthenticatorTests extends AbstractLdapIntegratio
    }
 
     public void testLocalPasswordComparisonSucceedsWithCorrectPassword() {
-        LdapUserDetails user = authenticator.authenticate("Bob", "bobspassword");
+        LdapUserDetails user = authenticator.authenticate("bob", "bobspassword");
         // check username is retrieved.
-        assertEquals("Bob", user.getUsername());
+        assertEquals("bob", user.getUsername());
         assertEquals("bobspassword", user.getPassword());
     }
 
@@ -107,16 +107,16 @@ public class PasswordComparisonAuthenticatorTests extends AbstractLdapIntegratio
     }
 
     public void testOnlySpecifiedAttributesAreRetrieved() throws Exception {
-        authenticator.setUserAttributes(new String[] {"userPassword"});
+        authenticator.setUserAttributes(new String[] {"uid", "userPassword"});
         authenticator.setPasswordEncoder(new PlaintextPasswordEncoder());
 
         LdapUserDetails user = authenticator.authenticate("Bob", "bobspassword");
-        assertEquals("Should have retrieved 1 attribute (userPassword)", 1, user.getAttributes().size());
+        assertEquals("Should have retrieved 2 attribute (uid, userPassword)", 2, user.getAttributes().size());
     }
 
    public void testLdapCompareSucceedsWithCorrectPassword() {
        // Don't retrieve the password
-       authenticator.setUserAttributes(new String[] {"cn"});
+       authenticator.setUserAttributes(new String[] {"uid"});
        // Bob has a plaintext password.
        authenticator.setPasswordEncoder(new PlaintextPasswordEncoder());
        authenticator.authenticate("bob", "bobspassword");
@@ -124,7 +124,7 @@ public class PasswordComparisonAuthenticatorTests extends AbstractLdapIntegratio
 
    public void testLdapCompareSucceedsWithShaEncodedPassword() {
        // Don't retrieve the password
-       authenticator.setUserAttributes(new String[] {"cn"});
+       authenticator.setUserAttributes(new String[] {"uid"});
        authenticator.authenticate("ben", "benspassword");
    }
 
@@ -145,10 +145,10 @@ public class PasswordComparisonAuthenticatorTests extends AbstractLdapIntegratio
     }
 
    public void testLdapCompareWithDifferentPasswordAttributeSucceeds() {
-       authenticator.setUserAttributes(new String[] {"cn"});
+       authenticator.setUserAttributes(new String[] {"uid"});
        authenticator.setPasswordEncoder(new PlaintextPasswordEncoder());
-       authenticator.setPasswordAttributeName("uid");
-       authenticator.authenticate("bob", "bob");
+       authenticator.setPasswordAttributeName("cn");
+       authenticator.authenticate("bob", "Bob Hamilton");
    }
 
     public void testWithUserSearch() {

+ 9 - 7
core/src/test/java/org/acegisecurity/userdetails/ldap/LdapUserDetailsMapperTests.java

@@ -42,10 +42,11 @@ public class LdapUserDetailsMapperTests extends TestCase {
         DirContextAdapter ctx = new DirContextAdapter();
 
         ctx.setAttributeValues("userRole", new String[] {"X", "Y", "Z"});
+        ctx.setAttributeValue("uid", "ani");
 
-        LdapUserDetailsImpl.Essence user = (LdapUserDetailsImpl.Essence) mapper.mapFromContext(ctx);
+        LdapUserDetailsImpl user = (LdapUserDetailsImpl) mapper.mapFromContext(ctx);
 
-        assertEquals(3, user.getGrantedAuthorities().length);
+        assertEquals(3, user.getAuthorities().length);
     }
 
     /**
@@ -60,11 +61,12 @@ public class LdapUserDetailsMapperTests extends TestCase {
         attrs.put(new BasicAttribute("userRole", "x"));
 
         DirContextAdapter ctx = new DirContextAdapter(attrs, new DistinguishedName("cn=someName"));
+        ctx.setAttributeValue("uid", "ani");
 
-        LdapUserDetailsImpl.Essence user = (LdapUserDetailsImpl.Essence) mapper.mapFromContext(ctx);
+        LdapUserDetailsImpl user = (LdapUserDetailsImpl) mapper.mapFromContext(ctx);
 
-        assertEquals(1, user.getGrantedAuthorities().length);
-        assertEquals("ROLE_X", user.getGrantedAuthorities()[0].getAuthority());
+        assertEquals(1, user.getAuthorities().length);
+        assertEquals("ROLE_X", user.getAuthorities()[0].getAuthority());
     }
 
 //    public void testNonStringRoleAttributeIsIgnoredByDefault() throws Exception {
@@ -90,9 +92,9 @@ public class LdapUserDetailsMapperTests extends TestCase {
         attrs.put(new BasicAttribute("myappsPassword", "mypassword".getBytes()));
 
         DirContextAdapter ctx = new DirContextAdapter(attrs, new DistinguishedName("cn=someName"));
+        ctx.setAttributeValue("uid", "ani");
 
-        LdapUserDetails user =
-                ((LdapUserDetailsImpl.Essence) mapper.mapFromContext(ctx)).createUserDetails();
+        LdapUserDetails user = (LdapUserDetailsImpl) mapper.mapFromContext(ctx);
 
         assertEquals("mypassword", user.getPassword());
     }