From 843d0e691092bdcb12091f2504ee49e26f1d510d Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Thu, 27 Nov 2008 21:08:01 +0000 Subject: [PATCH] SEC-985: Added hideUsernameNotFoundException property to LdapAuthenticationProvider and set default to true. --- .../security/ldap/LdapUserSearch.java | 4 +- .../ldap/LdapAuthenticationProvider.java | 14 ++- .../ldap/LdapAuthenticationProviderTests.java | 96 ++++++++++++------- 3 files changed, 80 insertions(+), 34 deletions(-) diff --git a/core/src/main/java/org/springframework/security/ldap/LdapUserSearch.java b/core/src/main/java/org/springframework/security/ldap/LdapUserSearch.java index a26769446b..0489316089 100644 --- a/core/src/main/java/org/springframework/security/ldap/LdapUserSearch.java +++ b/core/src/main/java/org/springframework/security/ldap/LdapUserSearch.java @@ -16,6 +16,7 @@ package org.springframework.security.ldap; import org.springframework.ldap.core.DirContextOperations; +import org.springframework.security.userdetails.UsernameNotFoundException; /** @@ -38,6 +39,7 @@ public interface LdapUserSearch { * @param username the login name supplied to the authentication service. * * @return a DirContextOperations object containing the user's full DN and requested attributes. + * @throws UsernameNotFoundException if no user with the supplied name could be located by the search. */ - DirContextOperations searchForUser(String username); + DirContextOperations searchForUser(String username) throws UsernameNotFoundException; } diff --git a/core/src/main/java/org/springframework/security/providers/ldap/LdapAuthenticationProvider.java b/core/src/main/java/org/springframework/security/providers/ldap/LdapAuthenticationProvider.java index 9c24724b33..eaca4041eb 100644 --- a/core/src/main/java/org/springframework/security/providers/ldap/LdapAuthenticationProvider.java +++ b/core/src/main/java/org/springframework/security/providers/ldap/LdapAuthenticationProvider.java @@ -28,6 +28,7 @@ import org.springframework.security.ldap.populator.DefaultLdapAuthoritiesPopulat import org.springframework.security.providers.AuthenticationProvider; import org.springframework.security.providers.UsernamePasswordAuthenticationToken; import org.springframework.security.userdetails.UserDetails; +import org.springframework.security.userdetails.UsernameNotFoundException; import org.springframework.security.userdetails.ldap.LdapUserDetailsMapper; import org.springframework.security.userdetails.ldap.UserDetailsContextMapper; import org.springframework.security.util.AuthorityUtils; @@ -137,6 +138,7 @@ public class LdapAuthenticationProvider implements AuthenticationProvider { private LdapAuthoritiesPopulator authoritiesPopulator; private UserDetailsContextMapper userDetailsContextMapper = new LdapUserDetailsMapper(); private boolean useAuthenticationRequestCredentials = true; + private boolean hideUserNotFoundExceptions = true; //~ Constructors =================================================================================================== @@ -193,6 +195,10 @@ public class LdapAuthenticationProvider implements AuthenticationProvider { return userDetailsContextMapper; } + public void setHideUserNotFoundExceptions(boolean hideUserNotFoundExceptions) { + this.hideUserNotFoundExceptions = hideUserNotFoundExceptions; + } + /** * Determines whether the supplied password will be used as the credentials in the successful authentication * token. If set to false, then the password will be obtained from the UserDetails object @@ -236,7 +242,13 @@ public class LdapAuthenticationProvider implements AuthenticationProvider { UserDetails user = userDetailsContextMapper.mapUserFromContext(userData, username, extraAuthorities); return createSuccessfulAuthentication(userToken, user); - + } catch (UsernameNotFoundException notFound) { + if (hideUserNotFoundExceptions) { + throw new BadCredentialsException(messages.getMessage( + "LdapAuthenticationProvider.badCredentials", "Bad credentials")); + } else { + throw notFound; + } } catch (NamingException ldapAccessFailure) { throw new AuthenticationServiceException(ldapAccessFailure.getMessage(), ldapAccessFailure); } diff --git a/core/src/test/java/org/springframework/security/providers/ldap/LdapAuthenticationProviderTests.java b/core/src/test/java/org/springframework/security/providers/ldap/LdapAuthenticationProviderTests.java index 18e03fb7ea..72b2e4a1d0 100644 --- a/core/src/test/java/org/springframework/security/providers/ldap/LdapAuthenticationProviderTests.java +++ b/core/src/test/java/org/springframework/security/providers/ldap/LdapAuthenticationProviderTests.java @@ -15,24 +15,28 @@ package org.springframework.security.providers.ldap; -import org.springframework.security.Authentication; -import org.springframework.security.BadCredentialsException; -import org.springframework.security.GrantedAuthority; -import org.springframework.security.GrantedAuthorityImpl; -import org.springframework.security.ldap.LdapAuthoritiesPopulator; -import org.springframework.security.providers.UsernamePasswordAuthenticationToken; -import org.springframework.security.userdetails.UserDetails; -import org.springframework.security.userdetails.ldap.LdapUserDetailsMapper; -import org.springframework.security.util.AuthorityUtils; -import org.springframework.ldap.core.DirContextAdapter; -import org.springframework.ldap.core.DirContextOperations; -import org.springframework.ldap.core.DistinguishedName; - -import junit.framework.TestCase; +import static org.junit.Assert.*; import java.util.ArrayList; import java.util.List; +import org.jmock.Expectations; +import org.jmock.Mockery; +import org.jmock.integration.junit4.JUnit4Mockery; +import org.junit.Test; +import org.springframework.ldap.core.DirContextAdapter; +import org.springframework.ldap.core.DirContextOperations; +import org.springframework.ldap.core.DistinguishedName; +import org.springframework.security.Authentication; +import org.springframework.security.BadCredentialsException; +import org.springframework.security.GrantedAuthority; +import org.springframework.security.ldap.LdapAuthoritiesPopulator; +import org.springframework.security.providers.UsernamePasswordAuthenticationToken; +import org.springframework.security.userdetails.UserDetails; +import org.springframework.security.userdetails.UsernameNotFoundException; +import org.springframework.security.userdetails.ldap.LdapUserDetailsMapper; +import org.springframework.security.util.AuthorityUtils; + /** * Tests {@link LdapAuthenticationProvider}. @@ -40,19 +44,12 @@ import java.util.List; * @author Luke Taylor * @version $Id$ */ -public class LdapAuthenticationProviderTests extends TestCase { - //~ Constructors =================================================================================================== - - public LdapAuthenticationProviderTests(String string) { - super(string); - } - - public LdapAuthenticationProviderTests() { - } +public class LdapAuthenticationProviderTests { + Mockery jmock = new JUnit4Mockery(); //~ Methods ======================================================================================================== - + @Test public void testSupportsUsernamePasswordAuthenticationToken() { LdapAuthenticationProvider ldapProvider = new LdapAuthenticationProvider(new MockAuthenticator(), new MockAuthoritiesPopulator()); @@ -60,6 +57,7 @@ public class LdapAuthenticationProviderTests extends TestCase { assertTrue(ldapProvider.supports(UsernamePasswordAuthenticationToken.class)); } + @Test public void testDefaultMapperIsSet() { LdapAuthenticationProvider ldapProvider = new LdapAuthenticationProvider(new MockAuthenticator(), new MockAuthoritiesPopulator()); @@ -67,6 +65,7 @@ public class LdapAuthenticationProviderTests extends TestCase { assertTrue(ldapProvider.getUserDetailsContextMapper() instanceof LdapUserDetailsMapper); } + @Test public void testEmptyOrNullUserNameThrowsException() { LdapAuthenticationProvider ldapProvider = new LdapAuthenticationProvider(new MockAuthenticator(), new MockAuthoritiesPopulator()); @@ -82,15 +81,46 @@ public class LdapAuthenticationProviderTests extends TestCase { } catch (BadCredentialsException expected) {} } - public void testEmptyPasswordIsRejected() { + @Test(expected=BadCredentialsException.class) + public void emptyPasswordIsRejected() { LdapAuthenticationProvider ldapProvider = new LdapAuthenticationProvider(new MockAuthenticator()); - try { - ldapProvider.authenticate(new UsernamePasswordAuthenticationToken("jen", "")); - fail("Expected BadCredentialsException for empty password"); - } catch (BadCredentialsException expected) {} + ldapProvider.authenticate(new UsernamePasswordAuthenticationToken("jen", "")); } - public void testNormalUsage() { + @Test + public void usernameNotFoundExceptionIsHiddenByDefault() { + final LdapAuthenticator authenticator = jmock.mock(LdapAuthenticator.class); + final UsernamePasswordAuthenticationToken joe = new UsernamePasswordAuthenticationToken("joe", "password"); + jmock.checking(new Expectations() {{ + oneOf(authenticator).authenticate(joe); will(throwException(new UsernameNotFoundException("nobody"))); + }}); + + LdapAuthenticationProvider provider = new LdapAuthenticationProvider(authenticator); + try { + provider.authenticate(joe); + fail(); + } catch (BadCredentialsException expected) { + if (expected instanceof UsernameNotFoundException) { + fail("Exception should have been hidden"); + } + } + } + + @Test(expected=UsernameNotFoundException.class) + public void usernameNotFoundExceptionIsNotHiddenIfConfigured() { + final LdapAuthenticator authenticator = jmock.mock(LdapAuthenticator.class); + final UsernamePasswordAuthenticationToken joe = new UsernamePasswordAuthenticationToken("joe", "password"); + jmock.checking(new Expectations() {{ + oneOf(authenticator).authenticate(joe); will(throwException(new UsernameNotFoundException("nobody"))); + }}); + + LdapAuthenticationProvider provider = new LdapAuthenticationProvider(authenticator); + provider.setHideUserNotFoundExceptions(false); + provider.authenticate(joe); + } + + @Test + public void normalUsage() { MockAuthoritiesPopulator populator = new MockAuthoritiesPopulator(); LdapAuthenticationProvider ldapProvider = new LdapAuthenticationProvider(new MockAuthenticator(), populator); LdapUserDetailsMapper userMapper = new LdapUserDetailsMapper(); @@ -116,7 +146,8 @@ public class LdapAuthenticationProviderTests extends TestCase { assertTrue(authorities.contains("ROLE_FROM_POPULATOR")); } - public void testPasswordIsSetFromUserDataIfUseAuthenticationRequestCredentialsIsFalse() { + @Test + public void passwordIsSetFromUserDataIfUseAuthenticationRequestCredentialsIsFalse() { LdapAuthenticationProvider ldapProvider = new LdapAuthenticationProvider(new MockAuthenticator(), new MockAuthoritiesPopulator()); ldapProvider.setUseAuthenticationRequestCredentials(false); @@ -127,7 +158,8 @@ public class LdapAuthenticationProviderTests extends TestCase { } - public void testUseWithNullAuthoritiesPopulatorReturnsCorrectRole() { + @Test + public void useWithNullAuthoritiesPopulatorReturnsCorrectRole() { LdapAuthenticationProvider ldapProvider = new LdapAuthenticationProvider(new MockAuthenticator()); LdapUserDetailsMapper userMapper = new LdapUserDetailsMapper(); userMapper.setRoleAttributes(new String[] {"ou"});