From cd352f665bce2bbf8963f6bd4cc40cb6d145291d Mon Sep 17 00:00:00 2001
From: Rob Winch <rwinch@gopivotal.com>
Date: Tue, 24 Feb 2015 21:37:08 -0600
Subject: [PATCH] SEC-1915: Polish

* Restore default search filter to remain passive
* Check the search filter in setSearchFilter
* Add additional tests
---
 ...veDirectoryLdapAuthenticationProvider.java |  7 ++--
 ...ectoryLdapAuthenticationProviderTests.java | 40 +++++++++++++++++--
 2 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/ldap/src/main/java/org/springframework/security/ldap/authentication/ad/ActiveDirectoryLdapAuthenticationProvider.java b/ldap/src/main/java/org/springframework/security/ldap/authentication/ad/ActiveDirectoryLdapAuthenticationProvider.java
index a4e00106c5..6a26c2a896 100644
--- a/ldap/src/main/java/org/springframework/security/ldap/authentication/ad/ActiveDirectoryLdapAuthenticationProvider.java
+++ b/ldap/src/main/java/org/springframework/security/ldap/authentication/ad/ActiveDirectoryLdapAuthenticationProvider.java
@@ -97,7 +97,7 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLda
     private final String rootDn;
     private final String url;
     private boolean convertSubErrorCodesToExceptions;
-    private String searchFilter = "(&(objectClass=user)(|(sAMAccountName={0})(userPrincipalName={0})))";
+    private String searchFilter = "(&(objectClass=user)(userPrincipalName={0}))";
 
     // Only used to allow tests to substitute a mock LdapContext
     ContextFactory contextFactory = new ContextFactory();
@@ -337,14 +337,15 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLda
      * The LDAP filter string to search for the user being authenticated.
      * Occurrences of {0} are replaced with the {@code username@domain}.
      * <p>
-     * Defaults to: {@code (&(objectClass=user)(|(sAMAccountName={0})(userPrincipalName={0})))}
+     * Defaults to: {@code (&(objectClass=user)(userPrincipalName={0}))}
      * </p>
      *
      * @param searchFilter the filter string
      *
-     * @since 3.2
+     * @since 3.2.6
      */
     public void setSearchFilter(String searchFilter) {
+        Assert.hasText(searchFilter,"searchFilter must have text");
         this.searchFilter = searchFilter;
     }
 
diff --git a/ldap/src/test/java/org/springframework/security/ldap/authentication/ad/ActiveDirectoryLdapAuthenticationProviderTests.java b/ldap/src/test/java/org/springframework/security/ldap/authentication/ad/ActiveDirectoryLdapAuthenticationProviderTests.java
index 9c1e5dab81..11935f28ae 100644
--- a/ldap/src/test/java/org/springframework/security/ldap/authentication/ad/ActiveDirectoryLdapAuthenticationProviderTests.java
+++ b/ldap/src/test/java/org/springframework/security/ldap/authentication/ad/ActiveDirectoryLdapAuthenticationProviderTests.java
@@ -46,10 +46,7 @@ import java.util.Hashtable;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
-import static org.mockito.Mockito.any;
-import static org.mockito.Mockito.eq;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
 import static org.springframework.security.ldap.authentication.ad.ActiveDirectoryLdapAuthenticationProvider.ContextFactory;
 
 /**
@@ -124,6 +121,41 @@ public class ActiveDirectoryLdapAuthenticationProviderTests {
         assertTrue(result.isAuthenticated());
     }
 
+    @Test
+    public void defaultSearchFilter() throws Exception {
+        //given
+        final String defaultSearchFilter = "(&(objectClass=user)(userPrincipalName={0}))";
+
+        DirContext ctx = mock(DirContext.class);
+        when(ctx.getNameInNamespace()).thenReturn("");
+
+        DirContextAdapter dca = new DirContextAdapter();
+        SearchResult sr = new SearchResult("CN=Joe Jannsen,CN=Users", dca, dca.getAttributes());
+        when(ctx.search(any(Name.class), eq(defaultSearchFilter), any(Object[].class), any(SearchControls.class)))
+                .thenReturn(new MockNamingEnumeration(sr));
+
+        ActiveDirectoryLdapAuthenticationProvider customProvider
+                = new ActiveDirectoryLdapAuthenticationProvider("mydomain.eu", "ldap://192.168.1.200/");
+        customProvider.contextFactory = createContextFactoryReturning(ctx);
+
+        //when
+        Authentication result = customProvider.authenticate(joe);
+
+        //then
+        assertTrue(result.isAuthenticated());
+        verify(ctx).search(any(DistinguishedName.class), eq(defaultSearchFilter), any(Object[].class), any(SearchControls.class));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void setSearchFilterNull() {
+        provider.setSearchFilter(null);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void setSearchFilterEmpty() {
+        provider.setSearchFilter(" ");
+    }
+
     @Test
     public void nullDomainIsSupportedIfAuthenticatingWithFullUserPrincipal() throws Exception {
         provider = new ActiveDirectoryLdapAuthenticationProvider(null, "ldap://192.168.1.200/");