ActiveDirectoryLdapAuthenticationProvider uses InternalAuthenticationServiceException

Closes gh-2884

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

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.AccountExpiredException;
 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 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends
 			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 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends
 					|| (e instanceof OperationNotSupportedException)) {
 				handleBindException(bindPrincipal, e);
 				throw badCredentials(e);
-			}
-			else {
+			} else {
 				throw LdapUtils.convertLdapException(e);
 			}
 		}
@@ -313,6 +317,12 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends
 		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 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends
 					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

ldap/src/test/java/org/springframework/security/ldap/authentication/ad/ActiveDirectoryLdapAuthenticationProviderTests.java

@@ -32,6 +32,7 @@ import org.springframework.security.authentication.AccountExpiredException;
 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 @@ import static org.springframework.security.ldap.authentication.ad.ActiveDirector
  * @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 class ActiveDirectoryLdapAuthenticationProviderTests {
 	}
 
 	@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 class ActiveDirectoryLdapAuthenticationProviderTests {
 			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);
 		}
 	}