Skip to content

Commit f7b33da

Browse files
dadikovirwinch
authored andcommitted
ActiveDirectoryLdapAuthenticationProvider uses InternalAuthenticationServiceException
Closes gh-2884
1 parent 0f29bee commit f7b33da

File tree

3 files changed

+45
-11
lines changed

3 files changed

+45
-11
lines changed

core/src/main/resources/org/springframework/security/messages.properties

+1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ DigestAuthenticationFilter.usernameNotFound=Username {0} not found
3131
JdbcDaoImpl.noAuthority=User {0} has no GrantedAuthority
3232
JdbcDaoImpl.notFound=User {0} not found
3333
LdapAuthenticationProvider.badCredentials=Bad credentials
34+
LdapAuthenticationProvider.badLdapConnection=Connection to LDAP server failed
3435
LdapAuthenticationProvider.credentialsExpired=User credentials have expired
3536
LdapAuthenticationProvider.disabled=User is disabled
3637
LdapAuthenticationProvider.expired=User account has expired

ldap/src/main/java/org/springframework/security/ldap/authentication/ad/ActiveDirectoryLdapAuthenticationProvider.java

+17-4
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package org.springframework.security.ldap.authentication.ad;
1717

1818
import org.springframework.dao.IncorrectResultSizeDataAccessException;
19+
import org.springframework.ldap.CommunicationException;
1920
import org.springframework.ldap.core.DirContextOperations;
2021
import org.springframework.ldap.core.DistinguishedName;
2122
import org.springframework.ldap.core.support.DefaultDirObjectFactory;
@@ -24,6 +25,7 @@
2425
import org.springframework.security.authentication.BadCredentialsException;
2526
import org.springframework.security.authentication.CredentialsExpiredException;
2627
import org.springframework.security.authentication.DisabledException;
28+
import org.springframework.security.authentication.InternalAuthenticationServiceException;
2729
import org.springframework.security.authentication.LockedException;
2830
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
2931
import org.springframework.security.core.GrantedAuthority;
@@ -142,12 +144,15 @@ protected DirContextOperations doAuthentication(
142144
UsernamePasswordAuthenticationToken auth) {
143145
String username = auth.getName();
144146
String password = (String) auth.getCredentials();
145-
146-
DirContext ctx = bindAsUser(username, password);
147+
DirContext ctx = null;
147148

148149
try {
150+
ctx = bindAsUser(username, password);
149151
return searchForUser(ctx, username);
150152
}
153+
catch (CommunicationException e) {
154+
throw badLdapConnection(e);
155+
}
151156
catch (NamingException e) {
152157
logger.error("Failed to locate directory entry for authenticated user: "
153158
+ username, e);
@@ -210,8 +215,7 @@ private DirContext bindAsUser(String username, String password) {
210215
|| (e instanceof OperationNotSupportedException)) {
211216
handleBindException(bindPrincipal, e);
212217
throw badCredentials(e);
213-
}
214-
else {
218+
} else {
215219
throw LdapUtils.convertLdapException(e);
216220
}
217221
}
@@ -313,6 +317,12 @@ private BadCredentialsException badCredentials(Throwable cause) {
313317
return (BadCredentialsException) badCredentials().initCause(cause);
314318
}
315319

320+
private InternalAuthenticationServiceException badLdapConnection(Throwable cause) {
321+
return new InternalAuthenticationServiceException(messages.getMessage(
322+
"LdapAuthenticationProvider.badLdapConnection",
323+
"Connection to LDAP server failed."), cause);
324+
}
325+
316326
private DirContextOperations searchForUser(DirContext context, String username)
317327
throws NamingException {
318328
SearchControls searchControls = new SearchControls();
@@ -327,6 +337,9 @@ private DirContextOperations searchForUser(DirContext context, String username)
327337
searchControls, searchRoot, searchFilter,
328338
new Object[] { bindPrincipal, username });
329339
}
340+
catch (CommunicationException ldapCommunicationException) {
341+
throw badLdapConnection(ldapCommunicationException);
342+
}
330343
catch (IncorrectResultSizeDataAccessException incorrectResults) {
331344
// Search should never return multiple results if properly configured - just
332345
// rethrow

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

+27-7
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.springframework.security.authentication.BadCredentialsException;
3333
import org.springframework.security.authentication.CredentialsExpiredException;
3434
import org.springframework.security.authentication.DisabledException;
35+
import org.springframework.security.authentication.InternalAuthenticationServiceException;
3536
import org.springframework.security.authentication.LockedException;
3637
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
3738
import org.springframework.security.core.Authentication;
@@ -58,6 +59,9 @@
5859
* @author Rob Winch
5960
*/
6061
public class ActiveDirectoryLdapAuthenticationProviderTests {
62+
public static final String EXISTING_LDAP_PROVIDER = "ldap://192.168.1.200/";
63+
public static final String NON_EXISTING_LDAP_PROVIDER = "ldap://192.168.1.201/";
64+
6165
@Rule
6266
public ExpectedException thrown = ExpectedException.none();
6367

@@ -378,16 +382,29 @@ public void errorWithNoSubcodeIsHandledCleanly() {
378382
}
379383

380384
@Test(expected = org.springframework.ldap.CommunicationException.class)
381-
public void nonAuthenticationExceptionIsConvertedToSpringLdapException() {
382-
provider.contextFactory = createContextFactoryThrowing(new CommunicationException(
383-
msg));
384-
provider.authenticate(joe);
385+
public void nonAuthenticationExceptionIsConvertedToSpringLdapException() throws Throwable {
386+
try {
387+
provider.contextFactory = createContextFactoryThrowing(new CommunicationException(
388+
msg));
389+
provider.authenticate(joe);
390+
} catch (InternalAuthenticationServiceException e) {
391+
// Since GH-8418 ldap communication exception is wrapped into InternalAuthenticationServiceException.
392+
// This test is about the wrapped exception, so we throw it.
393+
throw e.getCause();
394+
}
395+
}
396+
397+
@Test(expected = org.springframework.security.authentication.InternalAuthenticationServiceException.class )
398+
public void connectionExceptionIsWrappedInInternalException() throws Exception {
399+
ActiveDirectoryLdapAuthenticationProvider noneReachableProvider = new ActiveDirectoryLdapAuthenticationProvider(
400+
"mydomain.eu", NON_EXISTING_LDAP_PROVIDER, "dc=ad,dc=eu,dc=mydomain");
401+
noneReachableProvider.doAuthentication(joe);
385402
}
386403

387404
@Test
388405
public void rootDnProvidedSeparatelyFromDomainAlsoWorks() throws Exception {
389406
ActiveDirectoryLdapAuthenticationProvider provider = new ActiveDirectoryLdapAuthenticationProvider(
390-
"mydomain.eu", "ldap://192.168.1.200/", "dc=ad,dc=eu,dc=mydomain");
407+
"mydomain.eu", EXISTING_LDAP_PROVIDER, "dc=ad,dc=eu,dc=mydomain");
391408
checkAuthentication("dc=ad,dc=eu,dc=mydomain", provider);
392409

393410
}
@@ -413,8 +430,11 @@ public void contextEnvironmentPropertiesUsed() {
413430
provider.authenticate(joe);
414431
fail("CommunicationException was expected with a root cause of ClassNotFoundException");
415432
}
416-
catch (org.springframework.ldap.CommunicationException expected) {
417-
assertThat(expected.getRootCause()).isInstanceOf(ClassNotFoundException.class);
433+
catch (InternalAuthenticationServiceException expected) {
434+
assertThat(expected.getCause()).isInstanceOf(org.springframework.ldap.CommunicationException.class);
435+
org.springframework.ldap.CommunicationException cause =
436+
(org.springframework.ldap.CommunicationException) expected.getCause();
437+
assertThat(cause.getRootCause()).isInstanceOf(ClassNotFoundException.class);
418438
}
419439
}
420440

0 commit comments

Comments
 (0)