Browse Source

Support Handling Javax-based Bind Exceptions

Closes gh-3834
Josh Cummings 1 year ago
parent
commit
ef5c1a72db

+ 30 - 0
ldap/src/integration-test/java/org/springframework/security/ldap/authentication/BindAuthenticatorTests.java

@@ -16,12 +16,19 @@
 
 package org.springframework.security.ldap.authentication;
 
+import javax.naming.Name;
+import javax.naming.ldap.LdapContext;
+
+import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.ldap.AuthenticationException;
 import org.springframework.ldap.core.DirContextOperations;
+import org.springframework.ldap.core.support.BaseLdapPathContextSource;
+import org.springframework.ldap.support.LdapUtils;
 import org.springframework.security.authentication.BadCredentialsException;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.core.Authentication;
@@ -34,6 +41,10 @@ import org.springframework.test.context.junit.jupiter.SpringExtension;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
 
 /**
  * Tests for {@link BindAuthenticator}.
@@ -142,4 +153,23 @@ public class BindAuthenticatorTests {
 		assertThat(this.authenticator.getUserDns("Joe").get(0)).isEqualTo("cn=Joe,ou=people");
 	}
 
+	@Test
+	public void setAlsoHandleJavaxNamingBindExceptionsWhenTrueThenHandles() throws Exception {
+		BaseLdapPathContextSource contextSource = spy(this.contextSource);
+		BindAuthenticator authenticator = new BindAuthenticator(contextSource);
+		authenticator.setUserDnPatterns(new String[] { "uid={0},ou=people" });
+		LdapContext dirContext = mock(LdapContext.class);
+		given(dirContext.getAttributes(any(Name.class), any())).willThrow(new javax.naming.AuthenticationException("exception"));
+		Name fullDn = LdapUtils.newLdapName("uid=bob,ou=people").addAll(0, contextSource.getBaseLdapPath());
+		given(contextSource.getContext(fullDn.toString(), (String) this.bob.getCredentials())).willReturn(dirContext);
+		authenticator.setAlsoHandleJavaxNamingBindExceptions(true);
+		assertThatExceptionOfType(BadCredentialsException.class).isThrownBy(authenticateBob(authenticator));
+		authenticator.setAlsoHandleJavaxNamingBindExceptions(false);
+		assertThatExceptionOfType(AuthenticationException.class).isThrownBy(authenticateBob(authenticator))
+				.withCauseInstanceOf(javax.naming.AuthenticationException.class);
+	}
+
+	private ThrowingCallable authenticateBob(BindAuthenticator authenticator) {
+		return () -> authenticator.authenticate(this.bob);
+	}
 }

+ 31 - 8
ldap/src/main/java/org/springframework/security/ldap/authentication/BindAuthenticator.java

@@ -47,6 +47,8 @@ public class BindAuthenticator extends AbstractLdapAuthenticator {
 
 	private static final Log logger = LogFactory.getLog(BindAuthenticator.class);
 
+	private boolean alsoHandleJavaxNamingBindExceptions = false;
+
 	/**
 	 * Create an initialized instance using the {@link BaseLdapPathContextSource}
 	 * provided.
@@ -125,16 +127,13 @@ public class BindAuthenticator extends AbstractLdapAuthenticator {
 			// This will be thrown if an invalid user name is used and the method may
 			// be called multiple times to try different names, so we trap the exception
 			// unless a subclass wishes to implement more specialized behaviour.
-			if ((ex instanceof org.springframework.ldap.AuthenticationException)
-					|| (ex instanceof org.springframework.ldap.OperationNotSupportedException)) {
-				handleBindException(userDnStr, username, ex);
-			}
-			else {
-				throw ex;
-			}
+			handleIfBindException(userDnStr, username, ex);
 		}
 		catch (javax.naming.NamingException ex) {
-			throw LdapUtils.convertLdapException(ex);
+			if (!this.alsoHandleJavaxNamingBindExceptions) {
+				throw LdapUtils.convertLdapException(ex);
+			}
+			handleIfBindException(userDnStr, username, LdapUtils.convertLdapException(ex));
 		}
 		finally {
 			LdapUtils.closeContext(ctx);
@@ -142,6 +141,16 @@ public class BindAuthenticator extends AbstractLdapAuthenticator {
 		return null;
 	}
 
+	private void handleIfBindException(String dn, String username, org.springframework.ldap.NamingException naming) {
+		if ((naming instanceof org.springframework.ldap.AuthenticationException)
+				|| (naming instanceof org.springframework.ldap.OperationNotSupportedException)) {
+			handleBindException(dn, username, naming);
+		}
+		else {
+			throw naming;
+		}
+	}
+
 	/**
 	 * Allows subclasses to inspect the exception thrown by an attempt to bind with a
 	 * particular DN. The default implementation just reports the failure to the debug
@@ -151,4 +160,18 @@ public class BindAuthenticator extends AbstractLdapAuthenticator {
 		logger.trace(LogMessage.format("Failed to bind as %s", userDn), cause);
 	}
 
+	/**
+	 * Set whether javax-based bind exceptions should also be delegated to {@code #handleBindException}
+	 * (only Spring-based bind exceptions are handled by default)
+	 *
+	 * <p>For passivity reasons, defaults to {@code false}, though may change to {@code true}
+	 * in future releases.
+	 *
+	 * @param alsoHandleJavaxNamingBindExceptions - whether to delegate javax-based bind exceptions to
+	 * #handleBindException
+	 * @since 6.4
+	 */
+	public void setAlsoHandleJavaxNamingBindExceptions(boolean alsoHandleJavaxNamingBindExceptions) {
+		this.alsoHandleJavaxNamingBindExceptions = alsoHandleJavaxNamingBindExceptions;
+	}
 }