Skip to content

Commit

Permalink
ActiveDirectoryLdapAuthenticationProvider uses InternalAuthentication…
Browse files Browse the repository at this point in the history
…ServiceException

Closes gh-2884
  • Loading branch information
dadikovi authored and rwinch committed Apr 24, 2020
1 parent 52af399 commit f98db2d
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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();
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();

Expand Down Expand Up @@ -378,17 +382,29 @@ public void errorWithNoSubcodeIsHandledCleanly() throws Exception {
}

@Test(expected = org.springframework.ldap.CommunicationException.class)
public void nonAuthenticationExceptionIsConvertedToSpringLdapException()
throws Exception {
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);

}
Expand All @@ -414,8 +430,11 @@ public void contextEnvironmentPropertiesUsed() throws Exception {
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);
}
}

Expand Down

0 comments on commit f98db2d

Please sign in to comment.