[java-identity-provider] branch main updated: IDP-2369 - encode control chars in LDAP exception messages

[Daniel Fisher]

This is an automated email from the git hooks/post-receive script.

dfisher pushed a commit to branch main
in repository java-identity-provider.

View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=fe663461471b289d9e6bb3027f3fb6ccb0220464

The following commit(s) were added to refs/heads/main by this push:
     new fe6634614 IDP-2369 - encode control chars in LDAP exception messages
fe6634614 is described below

commit fe663461471b289d9e6bb3027f3fb6ccb0220464
Author: Daniel Fisher <dfisher at vt.edu>
AuthorDate: Tue Apr 15 08:24:15 2025 -0400

    IDP-2369 - encode control chars in LDAP exception messages
    
    https://shibboleth.atlassian.net/browse/IDP-2369
    
    Use getEncodedDiagnosticMessage instead of getDiagnosticMessage.
    Update unit tests.
---
 .../idp/authn/impl/LDAPCredentialValidator.java    | 11 +++---
 .../authn/impl/LDAPCredentialValidatorTest.java    | 44 ++++++++++++++++++++++
 2 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/LDAPCredentialValidator.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/LDAPCredentialValidator.java
index 4d75219f1..2f8b19c78 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/LDAPCredentialValidator.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/impl/LDAPCredentialValidator.java
@@ -181,7 +181,7 @@ public class LDAPCredentialValidator extends AbstractUsernamePasswordCredentialV
                       profileRequestContext,
                       authenticationContext,
                       String.format("%s:%s:%s", error != null ? error : "ACCOUNT_WARNING",
-                        response.getResultCode(), response.getDiagnosticMessage()),
+                        response.getResultCode(), response.getEncodedDiagnosticMessage()),
                       AuthnEventIds.ACCOUNT_WARNING);
                 }
             }
@@ -194,23 +194,24 @@ public class LDAPCredentialValidator extends AbstractUsernamePasswordCredentialV
                 || AuthenticationResultCode.INVALID_CREDENTIAL == response.getAuthenticationResultCode()) {
             eventToSignal = AuthnEventIds.INVALID_CREDENTIALS;
             authException = new LdapException(
-              String.format("%s:%s", response.getAuthenticationResultCode(), response.getDiagnosticMessage()));
+              String.format("%s:%s", response.getAuthenticationResultCode(), response.getEncodedDiagnosticMessage()));
         } else if (response.getAccountState() != null) {
             final AccountState state = response.getAccountState();
             eventToSignal = AuthnEventIds.ACCOUNT_ERROR;
             authException = new LdapException(
                 response.getResultCode(),
-                String.format("%s:%s:%s", state.getError(), response.getResultCode(), response.getDiagnosticMessage()));
+                String.format(
+                  "%s:%s:%s", state.getError(), response.getResultCode(), response.getEncodedDiagnosticMessage()));
         } else if (response.getResultCode() == ResultCode.INVALID_CREDENTIALS) {
             eventToSignal = AuthnEventIds.INVALID_CREDENTIALS;
             authException = new LdapException(
                 response.getResultCode(),
-                String.format("%s:%s", response.getResultCode(), response.getDiagnosticMessage()));
+                String.format("%s:%s", response.getResultCode(), response.getEncodedDiagnosticMessage()));
         } else {
             eventToSignal = AuthnEventIds.AUTHN_EXCEPTION;
             authException = new LdapException(
                 response.getResultCode(),
-                String.format("%s:%s", response.getResultCode(), response.getDiagnosticMessage()));
+                String.format("%s:%s", response.getResultCode(), response.getEncodedDiagnosticMessage()));
         }
 
         log.info("{} Login by '{}' failed", getLogPrefix(), username, authException);
diff --git a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/LDAPCredentialValidatorTest.java b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/LDAPCredentialValidatorTest.java
index 17d2ea2b4..0cf9cd95a 100644
--- a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/LDAPCredentialValidatorTest.java
+++ b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/LDAPCredentialValidatorTest.java
@@ -15,6 +15,7 @@
 package net.shibboleth.idp.authn.impl;
 
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
 
 import java.time.ZonedDateTime;
 import java.util.Collection;
@@ -23,8 +24,12 @@ import java.util.Map;
 import java.util.function.Function;
 import java.util.regex.Pattern;
 
+import org.ldaptive.BindResponse;
 import org.ldaptive.DefaultConnectionFactory;
+import org.ldaptive.LdapException;
+import org.ldaptive.ResultCode;
 import org.ldaptive.auth.AccountState;
+import org.ldaptive.auth.AuthenticationHandlerResponse;
 import org.ldaptive.auth.AuthenticationResponse;
 import org.ldaptive.auth.AuthenticationResultCode;
 import org.ldaptive.auth.Authenticator;
@@ -67,6 +72,12 @@ public class LDAPCredentialValidatorTest extends BaseAuthenticationContextTest {
 
     private static final String DATA_PATH = "/net/shibboleth/idp/authn/impl/";
 
+    static {
+        // property used to control the encoding of control characters in ldap exception messages
+        // this will be deprecated in a future release
+        System.setProperty("org.ldaptive.response.ENCODE_CNTRL_CHARS", "true");
+    }
+
     private LDAPCredentialValidator validator;
     
     private ValidateCredentials action;
@@ -477,6 +488,39 @@ public class LDAPCredentialValidatorTest extends BaseAuthenticationContextTest {
         Assert.assertTrue(awc.isClassifiedWarning("ExpiringPassword"));
     }
 
+    @Test public void testControlCharExceptionMsg() throws ComponentInitializationException {
+        final Authenticator errorAuthenticator = new Authenticator(
+          user -> "not-a-dn",
+          criteria -> new AuthenticationHandlerResponse(
+            BindResponse.builder()
+              .resultCode(ResultCode.INVALID_CREDENTIALS)
+              .diagnosticMessage("Message with control character: \0")
+              .build(),
+            AuthenticationResultCode.AUTHENTICATION_HANDLER_FAILURE,
+            null)
+        );
+
+        final LDAPCredentialValidator validator = new LDAPCredentialValidator();
+        validator.setId("ldaptest-exception-msg");
+        validator.setAuthenticator(errorAuthenticator);
+        validator.setUsernamePasswordContextLookupStrategy(context -> {
+            final UsernamePasswordContext cxt = new UsernamePasswordContext();
+            cxt.setUsername("PETER_THE_PRINCIPAL");
+            cxt.setPassword("changeit");
+            return cxt;
+        });
+        validator.initialize();
+
+        try {
+            validator.validate(new ProfileRequestContext(), new AuthenticationContext(), null, null);
+        } catch (Exception e) {
+            assertNotNull(e);
+            assertEquals(e.getClass(), LdapException.class);
+            assertNotNull(e.getMessage());
+            assertEquals(e.getMessage(), "INVALID_CREDENTIALS:Message with control character: %00");
+        }
+    }
+
     @Test public void testAuthorized() throws ComponentInitializationException {
         getMockHttpServletRequest(action).addParameter("username", "PETER_THE_PRINCIPAL");
         getMockHttpServletRequest(action).addParameter("password", "changeit");

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.