diff --git a/core/src/main/resources/org/springframework/security/messages.properties b/core/src/main/resources/org/springframework/security/messages.properties index 665a742002e..a84f259753f 100644 --- a/core/src/main/resources/org/springframework/security/messages.properties +++ b/core/src/main/resources/org/springframework/security/messages.properties @@ -31,6 +31,7 @@ DigestAuthenticationFilter.usernameNotFound=Username {0} not found JdbcDaoImpl.noAuthority=User {0} has no GrantedAuthority JdbcDaoImpl.notFound=User {0} not found LdapAuthenticationProvider.badCredentials=Bad credentials +LdapAuthenticationProvider.badLdapConnection=Connection to LDAP server failed LdapAuthenticationProvider.credentialsExpired=User credentials have expired LdapAuthenticationProvider.disabled=User is disabled LdapAuthenticationProvider.expired=User account has expired 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 91a41a0f29b..afcb2fa87e2 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 @@ -16,6 +16,7 @@ package org.springframework.security.ldap.authentication.ad; import org.springframework.dao.IncorrectResultSizeDataAccessException; +import org.springframework.ldap.CommunicationException; import org.springframework.ldap.core.DirContextOperations; import org.springframework.ldap.core.DistinguishedName; import org.springframework.ldap.core.support.DefaultDirObjectFactory; @@ -24,6 +25,7 @@ import org.springframework.security.authentication.BadCredentialsException; import org.springframework.security.authentication.CredentialsExpiredException; import org.springframework.security.authentication.DisabledException; +import org.springframework.security.authentication.InternalAuthenticationServiceException; import org.springframework.security.authentication.LockedException; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.GrantedAuthority; @@ -142,12 +144,15 @@ protected DirContextOperations doAuthentication( UsernamePasswordAuthenticationToken auth) { String username = auth.getName(); String password = (String) auth.getCredentials(); - - DirContext ctx = bindAsUser(username, password); + DirContext ctx = null; try { + ctx = bindAsUser(username, password); return searchForUser(ctx, username); } + catch (CommunicationException e) { + throw badLdapConnection(e); + } catch (NamingException e) { logger.error("Failed to locate directory entry for authenticated user: " + username, e); @@ -210,8 +215,7 @@ private DirContext bindAsUser(String username, String password) { || (e instanceof OperationNotSupportedException)) { handleBindException(bindPrincipal, e); throw badCredentials(e); - } - else { + } else { throw LdapUtils.convertLdapException(e); } } @@ -313,6 +317,12 @@ private BadCredentialsException badCredentials(Throwable cause) { return (BadCredentialsException) badCredentials().initCause(cause); } + private InternalAuthenticationServiceException badLdapConnection(Throwable cause) { + return new InternalAuthenticationServiceException(messages.getMessage( + "LdapAuthenticationProvider.badLdapConnection", + "Connection to LDAP server failed."), cause); + } + private DirContextOperations searchForUser(DirContext context, String username) throws NamingException { SearchControls searchControls = new SearchControls(); @@ -327,6 +337,9 @@ private DirContextOperations searchForUser(DirContext context, String username) searchControls, searchRoot, searchFilter, new Object[] { bindPrincipal, username }); } + catch (CommunicationException ldapCommunicationException) { + throw badLdapConnection(ldapCommunicationException); + } catch (IncorrectResultSizeDataAccessException incorrectResults) { // Search should never return multiple results if properly configured - just // rethrow 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 934b507d056..7302da38213 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 @@ -32,6 +32,7 @@ import org.springframework.security.authentication.BadCredentialsException; import org.springframework.security.authentication.CredentialsExpiredException; import org.springframework.security.authentication.DisabledException; +import org.springframework.security.authentication.InternalAuthenticationServiceException; import org.springframework.security.authentication.LockedException; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.Authentication; @@ -58,6 +59,9 @@ * @author Rob Winch */ public class ActiveDirectoryLdapAuthenticationProviderTests { + public static final String EXISTING_LDAP_PROVIDER = "ldap://192.168.1.200/"; + public static final String NON_EXISTING_LDAP_PROVIDER = "ldap://192.168.1.201/"; + @Rule public ExpectedException thrown = ExpectedException.none(); @@ -378,16 +382,29 @@ public void errorWithNoSubcodeIsHandledCleanly() { } @Test(expected = org.springframework.ldap.CommunicationException.class) - public void nonAuthenticationExceptionIsConvertedToSpringLdapException() { - provider.contextFactory = createContextFactoryThrowing(new CommunicationException( - msg)); - provider.authenticate(joe); + public void nonAuthenticationExceptionIsConvertedToSpringLdapException() throws Throwable { + try { + provider.contextFactory = createContextFactoryThrowing(new CommunicationException( + msg)); + provider.authenticate(joe); + } catch (InternalAuthenticationServiceException e) { + // Since GH-8418 ldap communication exception is wrapped into InternalAuthenticationServiceException. + // This test is about the wrapped exception, so we throw it. + throw e.getCause(); + } + } + + @Test(expected = org.springframework.security.authentication.InternalAuthenticationServiceException.class ) + public void connectionExceptionIsWrappedInInternalException() throws Exception { + ActiveDirectoryLdapAuthenticationProvider noneReachableProvider = new ActiveDirectoryLdapAuthenticationProvider( + "mydomain.eu", NON_EXISTING_LDAP_PROVIDER, "dc=ad,dc=eu,dc=mydomain"); + noneReachableProvider.doAuthentication(joe); } @Test public void rootDnProvidedSeparatelyFromDomainAlsoWorks() throws Exception { ActiveDirectoryLdapAuthenticationProvider provider = new ActiveDirectoryLdapAuthenticationProvider( - "mydomain.eu", "ldap://192.168.1.200/", "dc=ad,dc=eu,dc=mydomain"); + "mydomain.eu", EXISTING_LDAP_PROVIDER, "dc=ad,dc=eu,dc=mydomain"); checkAuthentication("dc=ad,dc=eu,dc=mydomain", provider); } @@ -413,8 +430,11 @@ public void contextEnvironmentPropertiesUsed() { provider.authenticate(joe); fail("CommunicationException was expected with a root cause of ClassNotFoundException"); } - catch (org.springframework.ldap.CommunicationException expected) { - assertThat(expected.getRootCause()).isInstanceOf(ClassNotFoundException.class); + catch (InternalAuthenticationServiceException expected) { + assertThat(expected.getCause()).isInstanceOf(org.springframework.ldap.CommunicationException.class); + org.springframework.ldap.CommunicationException cause = + (org.springframework.ldap.CommunicationException) expected.getCause(); + assertThat(cause.getRootCause()).isInstanceOf(ClassNotFoundException.class); } }