From 842ad929a456415ca85a342b83c8b37bb8419d8a Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Fri, 3 Feb 2006 19:53:08 +0000 Subject: [PATCH] Change search object to use constructor injection (SEC-165) . --- .../search/FilterBasedLdapUserSearch.java | 58 ++++++++++++------- .../DefaultInitialDirContextFactoryTests.java | 1 - .../FilterBasedLdapUserSearchTests.java | 32 +++++----- 3 files changed, 51 insertions(+), 40 deletions(-) diff --git a/core/src/main/java/org/acegisecurity/providers/ldap/search/FilterBasedLdapUserSearch.java b/core/src/main/java/org/acegisecurity/providers/ldap/search/FilterBasedLdapUserSearch.java index 039bb91813..dfef82bc97 100644 --- a/core/src/main/java/org/acegisecurity/providers/ldap/search/FilterBasedLdapUserSearch.java +++ b/core/src/main/java/org/acegisecurity/providers/ldap/search/FilterBasedLdapUserSearch.java @@ -83,6 +83,25 @@ public class FilterBasedLdapUserSearch implements LdapUserSearch { //~ Methods ================================================================ + public FilterBasedLdapUserSearch(String searchBase, + String searchFilter, + InitialDirContextFactory initialDirContextFactory) { + Assert.notNull(initialDirContextFactory, "initialDirContextFactory must not be null"); + Assert.notNull(searchFilter, "searchFilter must not be null."); + Assert.notNull(searchBase, "searchBase must not be null (an empty string is acceptable)."); + + this.searchFilter = searchFilter; + this.initialDirContextFactory = initialDirContextFactory; + this.searchBase = searchBase; + + if(searchBase.length() == 0) { + logger.info("SearchBase not set. Searches will be performed from the root: " + + initialDirContextFactory.getRootDn()); + } + } + + //~ Methods ================================================================ + /** * Return the LdapUserInfo containing the user's information, or null if * no SearchResult is found. @@ -95,6 +114,11 @@ public class FilterBasedLdapUserSearch implements LdapUserSearch { ctls.setTimeLimit( searchTimeLimit ); ctls.setSearchScope( searchScope ); + if (logger.isDebugEnabled()) { + logger.debug("Searching for user '" + username + "', in context " + ctx + + ", with user search " + this.toString()); + } + try { String[] args = new String[] { LdapUtils.escapeNameForFilter(username) }; @@ -106,13 +130,13 @@ public class FilterBasedLdapUserSearch implements LdapUserSearch { SearchResult searchResult = (SearchResult)results.next(); - if(results.hasMore()) { + if (results.hasMore()) { throw new BadCredentialsException("Expected a single user but search returned multiple results"); } StringBuffer userDn = new StringBuffer(searchResult.getName()); - if(searchBase.length() > 0) { + if (searchBase.length() > 0) { userDn.append(","); userDn.append(searchBase); } @@ -129,24 +153,6 @@ public class FilterBasedLdapUserSearch implements LdapUserSearch { } } - public void afterPropertiesSet() throws Exception { - Assert.notNull(initialDirContextFactory, "initialDirContextFactory must be set"); - Assert.notNull(searchFilter, "searchFilter must be set."); - - if(searchBase.equals("")) { - logger.info("No search base DN supplied. Search will be performed from the root: " + - initialDirContextFactory.getRootDn()); - } - } - - public void setInitialDirContextFactory(InitialDirContextFactory initialDirContextFactory) { - this.initialDirContextFactory = initialDirContextFactory; - } - - public void setSearchFilter(String searchFilter) { - this.searchFilter = searchFilter; - } - public void setSearchSubtree(boolean searchSubtree) { // this.searchSubtree = searchSubtree; this.searchScope = searchSubtree ? @@ -157,7 +163,15 @@ public class FilterBasedLdapUserSearch implements LdapUserSearch { this.searchTimeLimit = searchTimeLimit; } - public void setSearchBase(String searchBase) { - this.searchBase = searchBase; + public String toString() { + StringBuffer sb = new StringBuffer(); + + sb.append("[ searchFilter: '").append(searchFilter).append("', "); + sb.append("searchBase: '").append(searchBase).append("'"); + sb.append(", scope: ").append(searchScope == + SearchControls.SUBTREE_SCOPE ? "subtree" : "single-level, "); + sb.append("searchTimeLimit: ").append(searchTimeLimit).append(" ]"); + + return sb.toString(); } } diff --git a/core/src/test/java/org/acegisecurity/providers/ldap/DefaultInitialDirContextFactoryTests.java b/core/src/test/java/org/acegisecurity/providers/ldap/DefaultInitialDirContextFactoryTests.java index a3974e5ea1..563dd439c0 100644 --- a/core/src/test/java/org/acegisecurity/providers/ldap/DefaultInitialDirContextFactoryTests.java +++ b/core/src/test/java/org/acegisecurity/providers/ldap/DefaultInitialDirContextFactoryTests.java @@ -45,7 +45,6 @@ public class DefaultInitialDirContextFactoryTests extends AbstractLdapServerTest assertEquals("dc=acegisecurity,dc=org", idf.getRootDn()); } - public void testConnectionFailure() throws Exception { // Use the wrong port idf = new DefaultInitialDirContextFactory("ldap://localhost:60389"); diff --git a/core/src/test/java/org/acegisecurity/providers/ldap/search/FilterBasedLdapUserSearchTests.java b/core/src/test/java/org/acegisecurity/providers/ldap/search/FilterBasedLdapUserSearchTests.java index 9ccf580352..3cc450c180 100644 --- a/core/src/test/java/org/acegisecurity/providers/ldap/search/FilterBasedLdapUserSearchTests.java +++ b/core/src/test/java/org/acegisecurity/providers/ldap/search/FilterBasedLdapUserSearchTests.java @@ -3,7 +3,6 @@ package org.acegisecurity.providers.ldap.search; import org.acegisecurity.providers.ldap.AbstractLdapServerTestCase; import org.acegisecurity.providers.ldap.DefaultInitialDirContextFactory; import org.acegisecurity.providers.ldap.LdapUserInfo; -import org.acegisecurity.providers.ldap.search.FilterBasedLdapUserSearch; import org.acegisecurity.userdetails.UsernameNotFoundException; import org.acegisecurity.BadCredentialsException; @@ -15,7 +14,6 @@ import org.acegisecurity.BadCredentialsException; */ public class FilterBasedLdapUserSearchTests extends AbstractLdapServerTestCase { private DefaultInitialDirContextFactory dirCtxFactory; - private FilterBasedLdapUserSearch locator; public void setUp() throws Exception { dirCtxFactory = new DefaultInitialDirContextFactory(PROVIDER_URL); @@ -23,10 +21,6 @@ public class FilterBasedLdapUserSearchTests extends AbstractLdapServerTestCase { dirCtxFactory.setExtraEnvVars(EXTRA_ENV); dirCtxFactory.setManagerDn(MANAGER_USER); dirCtxFactory.setManagerPassword(MANAGER_PASSWORD); - locator = new FilterBasedLdapUserSearch(); - locator.setSearchSubtree(false); - locator.setSearchTimeLimit(0); - locator.setInitialDirContextFactory(dirCtxFactory); } public FilterBasedLdapUserSearchTests(String string) { @@ -38,26 +32,28 @@ public class FilterBasedLdapUserSearchTests extends AbstractLdapServerTestCase { } public void testBasicSearch() throws Exception { - locator.setSearchBase("ou=people"); - locator.setSearchFilter("(uid={0})"); - locator.afterPropertiesSet(); + FilterBasedLdapUserSearch locator = + new FilterBasedLdapUserSearch("ou=people", "(uid={0})", dirCtxFactory); LdapUserInfo bob = locator.searchForUser("bob"); + locator.setSearchSubtree(false); + locator.setSearchTimeLimit(0); // name is wrong with embedded apacheDS // assertEquals("uid=bob,ou=people,"+ROOT_DN, bob.getDn()); } public void testSubTreeSearchSucceeds() throws Exception { // Don't set the searchBase, so search from the root. - locator.setSearchFilter("(cn={0})"); + FilterBasedLdapUserSearch locator = + new FilterBasedLdapUserSearch("", "(cn={0})", dirCtxFactory); locator.setSearchSubtree(true); - locator.afterPropertiesSet(); + LdapUserInfo ben = locator.searchForUser("Ben Alex"); // assertEquals("uid=ben,ou=people,"+ROOT_DN, bob.getDn()); } public void testSearchForInvalidUserFails() { - locator.setSearchBase("ou=people"); - locator.setSearchFilter("(uid={0})"); + FilterBasedLdapUserSearch locator = + new FilterBasedLdapUserSearch("ou=people", "(uid={0})", dirCtxFactory); try { locator.searchForUser("Joe"); @@ -67,8 +63,8 @@ public class FilterBasedLdapUserSearchTests extends AbstractLdapServerTestCase { } public void testFailsOnMultipleMatches() { - locator.setSearchBase("ou=people"); - locator.setSearchFilter("(cn=*)"); + FilterBasedLdapUserSearch locator = + new FilterBasedLdapUserSearch("ou=people", "(cn=*)", dirCtxFactory); try { locator.searchForUser("Ignored"); @@ -80,8 +76,10 @@ public class FilterBasedLdapUserSearchTests extends AbstractLdapServerTestCase { // Try some funny business with filters. public void testExtraFilterPartToExcludeBob() throws Exception { - locator.setSearchBase("ou=people"); - locator.setSearchFilter("(&(cn=*)(!(|(uid={0})(uid=marissa))))"); + FilterBasedLdapUserSearch locator = + new FilterBasedLdapUserSearch("ou=people", + "(&(cn=*)(!(|(uid={0})(uid=marissa))))", + dirCtxFactory); // Search for bob, get back ben... LdapUserInfo ben = locator.searchForUser("bob");