[java-identity-provider] 01/01: Changes to support the ldaptive v2 API.

Daniel Fisher dfisher at vt.edu
Sat Oct 8 02:39:58 UTC 2022


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

dfisher pushed a commit to branch dev/ldaptive2-idp5
in repository java-identity-provider.

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

commit 6cae50084c0230331274a91321a068964c962a11
Author: Daniel Fisher <dfisher at vt.edu>
AuthorDate: Fri Oct 7 22:34:44 2022 -0400

    Changes to support the ldaptive v2 API.
    
    useSSL removed from LDAPAuthenticationFactoryBean.
---
 .../authn/AbstractTemplateSearchDnResolver.java    |  16 +-
 .../idp/authn/PooledTemplateSearchDnResolver.java  |  84 ----------
 .../idp/authn/TemplateSearchDnResolver.java        |  19 +--
 .../config/LDAPAuthenticationFactoryBean.java      | 182 +++++++++------------
 .../idp/authn/impl/LDAPCredentialValidator.java    | 118 ++++++-------
 .../principal/impl/LDAPPrincipalSerializer.java    |   4 +-
 .../DefaultAuthenticationResultSerializerTest.java |  11 +-
 .../authn/impl/LDAPCredentialValidatorTest.java    |  43 ++---
 .../idp/authn/impl/ValidateCredentialsTest.java    |   6 +-
 9 files changed, 173 insertions(+), 310 deletions(-)

diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractTemplateSearchDnResolver.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractTemplateSearchDnResolver.java
index 99975487b..7b9336e4a 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractTemplateSearchDnResolver.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/AbstractTemplateSearchDnResolver.java
@@ -27,8 +27,8 @@ import org.apache.velocity.app.event.EventCartridge;
 import org.apache.velocity.app.event.ReferenceInsertionEventHandler;
 import org.apache.velocity.context.Context;
 import org.apache.velocity.exception.VelocityException;
-import org.ldaptive.SearchFilter;
-import org.ldaptive.auth.AbstractSearchDnResolver;
+import org.ldaptive.FilterTemplate;
+import org.ldaptive.auth.SearchDnResolver;
 import org.ldaptive.auth.User;
 
 import net.shibboleth.shared.velocity.Template;
@@ -36,7 +36,7 @@ import net.shibboleth.shared.velocity.Template;
 /**
  * Base class for {@link Template} based search dn resolvers.
  */
-public abstract class AbstractTemplateSearchDnResolver extends AbstractSearchDnResolver {
+public abstract class AbstractTemplateSearchDnResolver extends SearchDnResolver {
 
     /** Template. */
     private final Template template;
@@ -66,8 +66,8 @@ public abstract class AbstractTemplateSearchDnResolver extends AbstractSearchDnR
         return template;
     }
 
-    @Override protected SearchFilter createSearchFilter(final User user) {
-        final SearchFilter filter = new SearchFilter();
+    @Override protected FilterTemplate createFilterTemplate(final User user) {
+        final FilterTemplate filter = new FilterTemplate();
         if (user != null && user.getContext() != null) {
             final VelocityContext context = (VelocityContext) user.getContext();
             final EventCartridge cartridge = new EventCartridge();
@@ -116,7 +116,7 @@ public abstract class AbstractTemplateSearchDnResolver extends AbstractSearchDnR
         }
 
         /**
-         * Returns {@link SearchFilter#encodeValue} if value is a string or byte array.
+         * Returns {@link FilterTemplate#encodeValue} if value is a string or byte array.
          * 
          * @param value to encode
          *
@@ -124,9 +124,9 @@ public abstract class AbstractTemplateSearchDnResolver extends AbstractSearchDnR
          */
         private Object encode(final Object value) {
             if (value instanceof String){ 
-                return SearchFilter.encodeValue((String) value);
+                return FilterTemplate.encodeValue((String) value);
             } else if (value instanceof byte[]) {
-                return SearchFilter.encodeValue((byte[]) value);
+                return FilterTemplate.encodeValue((byte[]) value);
             }
             return value;
         }
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/PooledTemplateSearchDnResolver.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/PooledTemplateSearchDnResolver.java
deleted file mode 100644
index 31220358d..000000000
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/PooledTemplateSearchDnResolver.java
+++ /dev/null
@@ -1,84 +0,0 @@
-/*
- * Licensed to the University Corporation for Advanced Internet Development,
- * Inc. (UCAID) under one or more contributor license agreements.  See the
- * NOTICE file distributed with this work for additional information regarding
- * copyright ownership. The UCAID licenses this file to You under the Apache
- * License, Version 2.0 (the "License"); you may not use this file except in
- * compliance with the License.  You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package net.shibboleth.idp.authn;
-
-import java.util.Arrays;
-import org.apache.velocity.app.VelocityEngine;
-import org.apache.velocity.exception.VelocityException;
-import org.ldaptive.Connection;
-import org.ldaptive.LdapException;
-import org.ldaptive.pool.PooledConnectionFactory;
-import org.ldaptive.pool.PooledConnectionFactoryManager;
-
-/**
- * {@link net.shibboleth.shared.velocity.Template}-based pooled search dn resolver.
- */
-public class PooledTemplateSearchDnResolver extends AbstractTemplateSearchDnResolver
-        implements PooledConnectionFactoryManager {
-
-    /** Connection factory. */
-    private PooledConnectionFactory factory;
-
-    /**
-     * Creates a new pooled template search DN resolver.
-     *
-     * @param engine velocity engine
-     * @param filter filter template
-     *
-     * @throws VelocityException if velocity is not configured properly or the filter template is invalid
-     */
-    public PooledTemplateSearchDnResolver(final VelocityEngine engine, final String filter) throws VelocityException {
-        super(engine, filter);
-    }
-
-    /**
-     * Creates a new pooled template search DN resolver.
-     *
-     * @param cf connection factory
-     * @param engine velocity engine
-     * @param filter filter template
-     *
-     * @throws VelocityException if velocity is not configured properly or the filter template is invalid
-     */
-    public PooledTemplateSearchDnResolver(final PooledConnectionFactory cf, final VelocityEngine engine,
-            final String filter) throws VelocityException {
-        super(engine, filter);
-        setConnectionFactory(cf);
-    }
-
-    @Override public PooledConnectionFactory getConnectionFactory() {
-        return factory;
-    }
-
-    @Override public void setConnectionFactory(final PooledConnectionFactory cf) {
-        factory = cf;
-    }
-
-    @Override protected Connection getConnection() throws LdapException {
-        return factory.getConnection();
-    }
-
-    @Override public String toString() {
-        return String.format(
-                "[%s@%d::factory=%s, templateName=%s, baseDn=%s, userFilter=%s, userFilterParameters=%s, "
-                        + "allowMultipleDns=%s, subtreeSearch=%s, derefAliases=%s]",
-                getClass().getName(), hashCode(), factory, getTemplate().getTemplateName(), getBaseDn(),
-                getUserFilter(), Arrays.toString(getUserFilterParameters()), getAllowMultipleDns(), getSubtreeSearch(),
-                getDerefAliases());
-    }
-}
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/TemplateSearchDnResolver.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/TemplateSearchDnResolver.java
index 5bd174777..5d57bff49 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/TemplateSearchDnResolver.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/TemplateSearchDnResolver.java
@@ -30,9 +30,6 @@ import org.ldaptive.LdapException;
  */
 public class TemplateSearchDnResolver extends AbstractTemplateSearchDnResolver implements ConnectionFactoryManager {
 
-    /** Connection factory. */
-    private ConnectionFactory factory;
-
     /**
      * Creates a new template search DN resolver.
      *
@@ -60,25 +57,11 @@ public class TemplateSearchDnResolver extends AbstractTemplateSearchDnResolver i
         setConnectionFactory(cf);
     }
 
-    @Override public ConnectionFactory getConnectionFactory() {
-        return factory;
-    }
-
-    @Override public void setConnectionFactory(final ConnectionFactory cf) {
-        factory = cf;
-    }
-
-    @Override protected Connection getConnection() throws LdapException {
-        final Connection conn = factory.getConnection();
-        conn.open();
-        return conn;
-    }
-
     @Override public String toString() {
         return String.format(
                 "[%s@%d::factory=%s, templateName=%s, baseDn=%s, userFilter=%s, userFilterParameters=%s, "
                         + "allowMultipleDns=%s, subtreeSearch=%s, derefAliases=%s]",
-                getClass().getName(), hashCode(), factory, getTemplate().getTemplateName(), getBaseDn(),
+                getClass().getName(), hashCode(), getConnectionFactory(), getTemplate().getTemplateName(), getBaseDn(),
                 getUserFilter(), Arrays.toString(getUserFilterParameters()), getAllowMultipleDns(), getSubtreeSearch(),
                 getDerefAliases());
     }
diff --git a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/config/LDAPAuthenticationFactoryBean.java b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/config/LDAPAuthenticationFactoryBean.java
index d003aaf22..9868e67ef 100644
--- a/idp-authn-api/src/main/java/net/shibboleth/idp/authn/config/LDAPAuthenticationFactoryBean.java
+++ b/idp-authn-api/src/main/java/net/shibboleth/idp/authn/config/LDAPAuthenticationFactoryBean.java
@@ -23,7 +23,6 @@ import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 import com.google.common.base.MoreObjects;
-import net.shibboleth.idp.authn.PooledTemplateSearchDnResolver;
 import net.shibboleth.idp.authn.TemplateSearchDnResolver;
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
 import net.shibboleth.shared.primitive.DeprecationSupport;
@@ -32,22 +31,21 @@ import net.shibboleth.shared.primitive.DeprecationSupport.ObjectType;
 import org.apache.velocity.app.VelocityEngine;
 import org.ldaptive.ActivePassiveConnectionStrategy;
 import org.ldaptive.BindConnectionInitializer;
-import org.ldaptive.BindRequest;
 import org.ldaptive.ConnectionConfig;
 import org.ldaptive.ConnectionInitializer;
 import org.ldaptive.Credential;
 import org.ldaptive.DefaultConnectionFactory;
-import org.ldaptive.LdapURL;
+import org.ldaptive.FilterTemplate;
+import org.ldaptive.PooledConnectionFactory;
 import org.ldaptive.RandomConnectionStrategy;
 import org.ldaptive.RoundRobinConnectionStrategy;
-import org.ldaptive.SearchFilter;
+import org.ldaptive.SearchConnectionValidator;
 import org.ldaptive.SearchRequest;
 import org.ldaptive.SearchScope;
+import org.ldaptive.SimpleBindRequest;
 import org.ldaptive.auth.Authenticator;
-import org.ldaptive.auth.BindAuthenticationHandler;
 import org.ldaptive.auth.FormatDnResolver;
-import org.ldaptive.auth.PooledBindAuthenticationHandler;
-import org.ldaptive.auth.PooledSearchEntryResolver;
+import org.ldaptive.auth.SimpleBindAuthenticationHandler;
 import org.ldaptive.auth.SearchEntryResolver;
 import org.ldaptive.auth.ext.ActiveDirectoryAuthenticationResponseHandler;
 import org.ldaptive.auth.ext.EDirectoryAuthenticationResponseHandler;
@@ -55,13 +53,9 @@ import org.ldaptive.auth.ext.FreeIPAAuthenticationResponseHandler;
 import org.ldaptive.auth.ext.PasswordExpirationAuthenticationResponseHandler;
 import org.ldaptive.auth.ext.PasswordPolicyAuthenticationRequestHandler;
 import org.ldaptive.auth.ext.PasswordPolicyAuthenticationResponseHandler;
-import org.ldaptive.pool.BindPassivator;
-import org.ldaptive.pool.BlockingConnectionPool;
+import org.ldaptive.pool.BindConnectionPassivator;
 import org.ldaptive.pool.IdlePruneStrategy;
-import org.ldaptive.pool.Passivator;
-import org.ldaptive.pool.PoolConfig;
-import org.ldaptive.pool.PooledConnectionFactory;
-import org.ldaptive.pool.SearchValidator;
+import org.ldaptive.pool.ConnectionPassivator;
 import org.ldaptive.ssl.AllowAnyHostnameVerifier;
 import org.ldaptive.ssl.CredentialConfig;
 import org.ldaptive.ssl.SslConfig;
@@ -200,9 +194,6 @@ public class LDAPAuthenticationFactoryBean extends AbstractFactoryBean<Authentic
   /** Whether to use startTLS for connections. */
   private boolean useStartTLS;
 
-  /** Whether to use LDAPS for connections. */
-  private boolean useSSL;
-
   /** Whether to use the allow-all hostname verifier. */
   private boolean disableHostnameVerification;
 
@@ -334,10 +325,6 @@ public class LDAPAuthenticationFactoryBean extends AbstractFactoryBean<Authentic
     useStartTLS = b;
   }
 
-  public void setUseSSL(final boolean b) {
-    useSSL = b;
-  }
-  
   public void setDisableHostnameVerification(final boolean b) {
       disableHostnameVerification = b;
   }
@@ -539,25 +526,26 @@ public class LDAPAuthenticationFactoryBean extends AbstractFactoryBean<Authentic
     }
     config.setSslConfig(createSslConfig());
     if (initializer != null) {
-      config.setConnectionInitializer(initializer);
+      config.setConnectionInitializers(initializer);
     }
     return config;
   }
 
   /**
-   * Returns a new blocking connection pool. Wires a {@link SearchValidator} by default.
+   * Returns a new pooled connection factory. Wires a {@link SearchConnectionValidator} by default.
    *
    * @param name of the connection pool
    * @param config to assign to the pool
    *
    * @return new blocking connection pool
    */
-  protected BlockingConnectionPool createConnectionPool(final String name, final ConnectionConfig config) {
-    return createConnectionPool(name, config, new SearchValidator());
+  protected PooledConnectionFactory createPooledConnectionFactory(final String name, final ConnectionConfig config) {
+    return createPooledConnectionFactory(
+      name, config, SearchConnectionValidator.builder().period(validatePeriod).build());
   }
 
   /**
-   * Returns a new blocking connection pool using the supplied search validator.
+   * Returns a new pooled connection factory using the supplied search validator.
    *
    * @param name of the connection pool
    * @param config to assign to the pool
@@ -565,13 +553,13 @@ public class LDAPAuthenticationFactoryBean extends AbstractFactoryBean<Authentic
    *
    * @return new blocking connection pool
    */
-  protected BlockingConnectionPool createConnectionPool(final String name, final ConnectionConfig config,
-                                                        final SearchValidator validator) {
-    return createConnectionPool(name, config, validator, null);
+  protected PooledConnectionFactory createPooledConnectionFactory(
+    final String name, final ConnectionConfig config, final SearchConnectionValidator validator) {
+    return createPooledConnectionFactory(name, config, validator, null);
   }
 
   /**
-   * Returns a new blocking connection pool using the supplied search validator and passivator type. Note that a {@link
+   * Returns a new pooled connection factory using the supplied search validator and passivator. Note that a {@link
    * PassivatorType#BIND} uses the configured {@link #bindDn} and {@link #bindDnCredential}.
    *
    * @param name of the connection pool
@@ -581,28 +569,30 @@ public class LDAPAuthenticationFactoryBean extends AbstractFactoryBean<Authentic
    *
    * @return new blocking connection pool
    */
-  protected BlockingConnectionPool createConnectionPool(final String name, final ConnectionConfig config,
-                                                        final SearchValidator validator, final Passivator passivator) {
-    final PoolConfig poolConfig = new PoolConfig();
-    poolConfig.setMinPoolSize(minPoolSize);
-    poolConfig.setMaxPoolSize(maxPoolSize);
-    poolConfig.setValidateOnCheckOut(validateOnCheckout);
-    poolConfig.setValidatePeriodically(validatePeriodically);
-    poolConfig.setValidatePeriod(validatePeriod);
-    final BlockingConnectionPool pool = new BlockingConnectionPool();
-    pool.setName(name);
-    pool.setBlockWaitTime(blockWaitTime);
-    pool.setPoolConfig(poolConfig);
-    pool.setPruneStrategy(new IdlePruneStrategy(prunePeriod, idleTime));
-    pool.setValidator(validator);
-    pool.setPassivator(passivator);
-    pool.setFailFastInitialize(false);
-    pool.setConnectionFactory(new DefaultConnectionFactory(config));
-    pool.initialize();
-    return pool;
-  }
-
-  protected SearchValidator createSearchValidator(final String baseDn, final String filter) {
+  protected PooledConnectionFactory createPooledConnectionFactory(
+    final String name,
+    final ConnectionConfig config,
+    final SearchConnectionValidator validator,
+    final ConnectionPassivator passivator) {
+    final PooledConnectionFactory factory = new PooledConnectionFactory();
+    factory.setConnectionConfig(config);
+    factory.setMinPoolSize(minPoolSize);
+    factory.setMaxPoolSize(maxPoolSize);
+    factory.setValidateOnCheckOut(validateOnCheckout);
+    factory.setValidatePeriodically(validatePeriodically);
+    factory.setName(name);
+    factory.setBlockWaitTime(blockWaitTime);
+    factory.setPruneStrategy(new IdlePruneStrategy(prunePeriod, idleTime));
+    factory.setValidator(validator);
+    if (passivator != null) {
+      factory.setPassivator(passivator);
+    }
+    factory.setFailFastInitialize(false);
+    factory.initialize();
+    return factory;
+  }
+
+  protected SearchConnectionValidator createSearchConnectionValidator(final String baseDn, final String filter) {
     final SearchRequest searchRequest = new SearchRequest();
     searchRequest.setReturnAttributes("1.1");
     searchRequest.setSearchScope(SearchScope.OBJECT);
@@ -612,22 +602,22 @@ public class LDAPAuthenticationFactoryBean extends AbstractFactoryBean<Authentic
     } else {
       searchRequest.setBaseDn("");
     }
-    final SearchFilter searchFilter = new SearchFilter();
+    final FilterTemplate searchFilter = new FilterTemplate();
     if (filter != null) {
       searchFilter.setFilter(filter);
     } else {
       searchFilter.setFilter("(objectClass=*)");
     }
-    searchRequest.setSearchFilter(searchFilter);
-    return new SearchValidator(searchRequest);
+    searchRequest.setFilter(searchFilter);
+    return SearchConnectionValidator.builder().request(searchRequest).period(validatePeriod).build();
   }
 
-  protected Passivator createPoolPassivator(final PassivatorType type) {
+  protected ConnectionPassivator createConnectionPassivator(final PassivatorType type) {
     switch(type) {
       case BIND:
-        return new BindPassivator(new BindRequest(bindDn, new Credential(bindDnCredential)));
+        return new BindConnectionPassivator(new SimpleBindRequest(bindDn, new Credential(bindDnCredential)));
       case ANONYMOUS_BIND:
-        return new BindPassivator();
+        return new BindConnectionPassivator();
       case NONE:
       default:
         return null;
@@ -637,30 +627,18 @@ public class LDAPAuthenticationFactoryBean extends AbstractFactoryBean<Authentic
 // Checkstyle: CyclomaticComplexity|MethodLength OFF
   @Override
   protected Authenticator createInstance() throws Exception {
-    // check for deprecated useSSL property
-    if (useSSL) {
-      DeprecationSupport.warn(ObjectType.PROPERTY, "useSSL", "LDAP authentication",
-              "use of ldaps:// scheme in connection URL");
-      final LdapURL url = new LdapURL(ldapUrl);
-      for (final String s : url.getHostnamesWithSchemeAndPort()) {
-        if (!s.startsWith("ldaps://")) {
-          throw new IllegalArgumentException("useSSL property specified but URL scheme is not ldaps:// for " + s);
-        }
-      }
-    }
     final Authenticator authenticator = new Authenticator();
     if (disablePooling) {
       authenticator.setAuthenticationHandler(
-        new BindAuthenticationHandler(new DefaultConnectionFactory(createConnectionConfig())));
+        new SimpleBindAuthenticationHandler(new DefaultConnectionFactory(createConnectionConfig())));
     } else {
       authenticator.setAuthenticationHandler(
-        new PooledBindAuthenticationHandler(
-          new PooledConnectionFactory(
-            createConnectionPool(
-              "bind-pool",
-              createConnectionConfig(),
-              createSearchValidator(validateDn, validateFilter),
-              createPoolPassivator(bindPoolPassivatorType)))));
+        new SimpleBindAuthenticationHandler(
+          createPooledConnectionFactory(
+            "bind-pool",
+            createConnectionConfig(),
+            createSearchConnectionValidator(validateDn, validateFilter),
+            createConnectionPassivator(bindPoolPassivatorType))));
     }
     switch(authenticatorType) {
     case BIND_SEARCH:
@@ -674,16 +652,15 @@ public class LDAPAuthenticationFactoryBean extends AbstractFactoryBean<Authentic
             createConnectionConfig(new BindConnectionInitializer(bindDn, new Credential(bindDnCredential)))));
         authenticator.setDnResolver(bindSearchDnResolver);
       } else {
-        final PooledTemplateSearchDnResolver bindSearchDnResolver =
-          new PooledTemplateSearchDnResolver(velocityEngine, userFilter);
+        final TemplateSearchDnResolver bindSearchDnResolver =
+          new TemplateSearchDnResolver(velocityEngine, userFilter);
         bindSearchDnResolver.setBaseDn(baseDn);
         bindSearchDnResolver.setSubtreeSearch(subtreeSearch);
         bindSearchDnResolver.setConnectionFactory(
-          new PooledConnectionFactory(
-            createConnectionPool(
-              "dn-search-pool",
-              createConnectionConfig(new BindConnectionInitializer(bindDn, new Credential(bindDnCredential))),
-              createSearchValidator(validateDn, validateFilter))));
+          createPooledConnectionFactory(
+            "dn-search-pool",
+            createConnectionConfig(new BindConnectionInitializer(bindDn, new Credential(bindDnCredential))),
+            createSearchConnectionValidator(validateDn, validateFilter)));
         authenticator.setDnResolver(bindSearchDnResolver);
       }
       authenticator.setResolveEntryOnFailure(resolveEntryOnFailure);
@@ -695,7 +672,7 @@ public class LDAPAuthenticationFactoryBean extends AbstractFactoryBean<Authentic
     case AD:
       authenticator.setDnResolver(new FormatDnResolver(dnFormat));
       authenticator.setResolveEntryOnFailure(resolveEntryOnFailure);
-      authenticator.setAuthenticationResponseHandlers(new ActiveDirectoryAuthenticationResponseHandler());
+      authenticator.setResponseHandlers(new ActiveDirectoryAuthenticationResponseHandler());
       break;
     case ANON_SEARCH:
       if (disablePooling) {
@@ -706,16 +683,15 @@ public class LDAPAuthenticationFactoryBean extends AbstractFactoryBean<Authentic
         anonSearchDnResolver.setConnectionFactory(new DefaultConnectionFactory(createConnectionConfig()));
         authenticator.setDnResolver(anonSearchDnResolver);
       } else {
-        final PooledTemplateSearchDnResolver anonSearchDnResolver =
-          new PooledTemplateSearchDnResolver(velocityEngine, userFilter);
+        final TemplateSearchDnResolver anonSearchDnResolver =
+          new TemplateSearchDnResolver(velocityEngine, userFilter);
         anonSearchDnResolver.setBaseDn(baseDn);
         anonSearchDnResolver.setSubtreeSearch(subtreeSearch);
         anonSearchDnResolver.setConnectionFactory(
-          new PooledConnectionFactory(
-            createConnectionPool(
-              "dn-search-pool",
-              createConnectionConfig(),
-              createSearchValidator(validateDn, validateFilter))));
+          createPooledConnectionFactory(
+            "dn-search-pool",
+            createConnectionConfig(),
+            createSearchConnectionValidator(validateDn, validateFilter)));
         authenticator.setDnResolver(anonSearchDnResolver);
       }
       authenticator.setResolveEntryOnFailure(resolveEntryOnFailure);
@@ -732,28 +708,27 @@ public class LDAPAuthenticationFactoryBean extends AbstractFactoryBean<Authentic
             createConnectionConfig(new BindConnectionInitializer(bindDn, new Credential(bindDnCredential)))));
         authenticator.setEntryResolver(searchEntryResolver);
       } else {
-        final PooledSearchEntryResolver searchEntryResolver = new PooledSearchEntryResolver();
+        final SearchEntryResolver searchEntryResolver = new SearchEntryResolver();
         searchEntryResolver.setConnectionFactory(
-          new PooledConnectionFactory(
-            createConnectionPool(
-              "entry-search-pool",
-              createConnectionConfig(new BindConnectionInitializer(bindDn, new Credential(bindDnCredential))),
-              createSearchValidator(validateDn, validateFilter))));
+          createPooledConnectionFactory(
+            "entry-search-pool",
+            createConnectionConfig(new BindConnectionInitializer(bindDn, new Credential(bindDnCredential))),
+            createSearchConnectionValidator(validateDn, validateFilter)));
         authenticator.setEntryResolver(searchEntryResolver);
       }
     }
 
     if (usePasswordPolicy) {
-      authenticator.setAuthenticationRequestHandlers(new PasswordPolicyAuthenticationRequestHandler());
-      authenticator.setAuthenticationResponseHandlers(new PasswordPolicyAuthenticationResponseHandler());
+      authenticator.setRequestHandlers(new PasswordPolicyAuthenticationRequestHandler());
+      authenticator.setResponseHandlers(new PasswordPolicyAuthenticationResponseHandler());
     } else if (usePasswordExpiration) {
-      authenticator.setAuthenticationResponseHandlers(new PasswordExpirationAuthenticationResponseHandler());
+      authenticator.setResponseHandlers(new PasswordExpirationAuthenticationResponseHandler());
     } else if (isActiveDirectory) {
-      authenticator.setAuthenticationResponseHandlers(new ActiveDirectoryAuthenticationResponseHandler(accountStateExpirationPeriod, accountStateWarningPeriod));
+      authenticator.setResponseHandlers(new ActiveDirectoryAuthenticationResponseHandler(accountStateExpirationPeriod, accountStateWarningPeriod));
     } else if (isEDirectory) {
-      authenticator.setAuthenticationResponseHandlers(new EDirectoryAuthenticationResponseHandler(accountStateWarningPeriod));
+      authenticator.setResponseHandlers(new EDirectoryAuthenticationResponseHandler(accountStateWarningPeriod));
     } else if (isFreeIPA) {
-      authenticator.setAuthenticationResponseHandlers(new FreeIPAAuthenticationResponseHandler(accountStateExpirationPeriod, accountStateWarningPeriod, accountStateLoginFailures));
+      authenticator.setResponseHandlers(new FreeIPAAuthenticationResponseHandler(accountStateExpirationPeriod, accountStateWarningPeriod, accountStateLoginFailures));
     }
     log.debug("Created {} from {}", authenticator, this);
     return authenticator;
@@ -768,7 +743,6 @@ public class LDAPAuthenticationFactoryBean extends AbstractFactoryBean<Authentic
             .add("connectionStrategyType", connectionStrategyType)
             .add("ldapUrl", ldapUrl)
             .add("useStartTLS", useStartTLS)
-            .add("useSSL", useSSL)
             .add("disableHostnameVerification", disableHostnameVerification)
             .add("connectTimeout", connectTimeout)
             .add("responseTimeout", responseTimeout)
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 740532a62..f55aee18b 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
@@ -33,6 +33,7 @@ import org.ldaptive.auth.AuthenticationResponse;
 import org.ldaptive.auth.AuthenticationResultCode;
 import org.ldaptive.auth.Authenticator;
 import org.ldaptive.auth.User;
+import org.ldaptive.handler.LdapEntryHandler;
 import org.ldaptive.jaas.LdapPrincipal;
 import org.opensaml.profile.context.ProfileRequestContext;
 import org.slf4j.Logger;
@@ -137,70 +138,71 @@ public class LDAPCredentialValidator extends AbstractUsernamePasswordCredentialV
         
         final String username = usernamePasswordContext.getTransformedUsername();
         
-        String eventToSignal = AuthnEventIds.INVALID_CREDENTIALS;
-        
-        // The error handling is squonky. We log at info to generically record the failure.
-        // Known conditions are not explicitly logged but are wrapped with an exception and
-        // reported out to the caller. Last ditch, an exception is logged on warn and then
-        // reported out.
-        
+        log.debug("{} Attempting to authenticate user {}", getLogPrefix(), username);
+        final VelocityContext context = new VelocityContext();
+        context.put("usernamePasswordContext", usernamePasswordContext);
+        final char[] password = passwordLookupStrategy != null ?
+          passwordLookupStrategy.apply(profileRequestContext) :
+          usernamePasswordContext.getPassword().toCharArray();
+        final AuthenticationRequest request = new AuthenticationRequest(
+          new User(username, context), new Credential(password), returnAttributes);
+        final AuthenticationResponse response;
         try {
-            log.debug("{} Attempting to authenticate user {}", getLogPrefix(), username);
-            final VelocityContext context = new VelocityContext();
-            context.put("usernamePasswordContext", usernamePasswordContext);
-            final char[] password = passwordLookupStrategy != null ?
-                    passwordLookupStrategy.apply(profileRequestContext) :
-                        usernamePasswordContext.getPassword().toCharArray();
-            final AuthenticationRequest request = new AuthenticationRequest(
-                    new User(username, context), new Credential(password), returnAttributes);
-            final AuthenticationResponse response = authenticator.authenticate(request);
-            log.trace("{} Authentication response {}", getLogPrefix(), response);
-            if (response.getResult()) {
-                log.info("{} Login by '{}' succeeded", getLogPrefix(), username);
-                authenticationContext.getSubcontext(
-                        LDAPResponseContext.class, true).setAuthenticationResponse(response);
-                if (response.getAccountState() != null) {
-                    final AccountState.Error error = response.getAccountState().getError();
-                    if (warningHandler != null) {
-                        warningHandler.handleWarning(
-                                profileRequestContext,
-                                authenticationContext,
-                                String.format("%s:%s:%s", error != null ? error : "ACCOUNT_WARNING",
-                                        response.getResultCode(), response.getMessage()),
-                                AuthnEventIds.ACCOUNT_WARNING);
-                    }
-                }
-                return populateSubject(usernamePasswordContext, response);
-            }
-            
-            authenticationContext.getSubcontext(
-                    LDAPResponseContext.class, true).setAuthenticationResponse(response);
-            if (AuthenticationResultCode.DN_RESOLUTION_FAILURE == response.getAuthenticationResultCode()
-                    || AuthenticationResultCode.INVALID_CREDENTIAL == response.getAuthenticationResultCode()) {
-                throw new LdapException(
-                        String.format("%s:%s", response.getAuthenticationResultCode(), response.getMessage()));
-            } else if (response.getAccountState() != null) {
-                final AccountState state = response.getAccountState();
-                eventToSignal = AuthnEventIds.ACCOUNT_ERROR;
-                throw new LdapException(
-                        String.format("%s:%s:%s", state.getError(), response.getResultCode(), response.getMessage())
-                        );
-            } else if (response.getResultCode() == ResultCode.INVALID_CREDENTIALS) {
-                throw new LdapException(String.format("%s:%s", response.getResultCode(), response.getMessage()));
-            } else {
-                eventToSignal = AuthnEventIds.AUTHN_EXCEPTION;
-                final LdapException e =
-                        new LdapException(response.getMessage(), response.getResultCode(), response.getMatchedDn(),
-                        response.getControls(), response.getReferralURLs(), response.getMessageId());
-                throw e;
-            }
+            // authenticator should only throw for communication errors
+            response = authenticator.authenticate(request);
         } catch (final LdapException e) {
-            log.info("{} Login by '{}' failed", getLogPrefix(), username, e);
+            log.error("{} Error attempting LDAP authentication for '{}'", getLogPrefix(), username, e);
             if (errorHandler != null) {
-                errorHandler.handleError(profileRequestContext, authenticationContext, e, eventToSignal);
+                errorHandler.handleError(
+                    profileRequestContext, authenticationContext, e, AuthnEventIds.AUTHN_EXCEPTION);
             }
             throw e;
         }
+
+        log.debug("{} Authentication response {}", getLogPrefix(), response);
+        authenticationContext.getSubcontext(LDAPResponseContext.class, true).setAuthenticationResponse(response);
+        if (response.isSuccess()) {
+            log.info("{} Login by '{}' succeeded", getLogPrefix(), username);
+            if (response.getAccountState() != null) {
+                final AccountState.Error error = response.getAccountState().getError();
+                if (warningHandler != null) {
+                    warningHandler.handleWarning(
+                      profileRequestContext,
+                      authenticationContext,
+                      String.format("%s:%s:%s", error != null ? error : "ACCOUNT_WARNING",
+                        response.getResultCode(), response.getDiagnosticMessage()),
+                      AuthnEventIds.ACCOUNT_WARNING);
+                }
+            }
+            return populateSubject(usernamePasswordContext, response);
+        }
+
+        String eventToSignal;
+        LdapException authException;
+        if (AuthenticationResultCode.DN_RESOLUTION_FAILURE == response.getAuthenticationResultCode()
+                || AuthenticationResultCode.INVALID_CREDENTIAL == response.getAuthenticationResultCode()) {
+            eventToSignal = AuthnEventIds.INVALID_CREDENTIALS;
+            authException = new LdapException(
+              String.format("%s:%s", response.getAuthenticationResultCode(), response.getDiagnosticMessage()));
+        } else if (response.getAccountState() != null) {
+            final AccountState state = response.getAccountState();
+            eventToSignal = AuthnEventIds.ACCOUNT_ERROR;
+            authException = new LdapException(
+                String.format("%s:%s:%s", state.getError(), response.getResultCode(), response.getDiagnosticMessage()));
+        } else if (response.getResultCode() == ResultCode.INVALID_CREDENTIALS) {
+            eventToSignal = AuthnEventIds.INVALID_CREDENTIALS;
+            authException = new LdapException(
+                String.format("%s:%s", response.getResultCode(), response.getDiagnosticMessage()));
+        } else {
+            eventToSignal = AuthnEventIds.AUTHN_EXCEPTION;
+            authException = new LdapException(response);
+        }
+
+        log.info("{} Login by '{}' failed", getLogPrefix(), username, authException);
+        if (errorHandler != null) {
+            errorHandler.handleError(profileRequestContext, authenticationContext, authException, eventToSignal);
+        }
+        throw authException;
     }
 // Checkstyle: CyclomaticComplexity ON
 
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/principal/impl/LDAPPrincipalSerializer.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/principal/impl/LDAPPrincipalSerializer.java
index 38a174b60..fcca182ac 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/principal/impl/LDAPPrincipalSerializer.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/principal/impl/LDAPPrincipalSerializer.java
@@ -135,9 +135,9 @@ public class LDAPPrincipalSerializer extends AbstractPrincipalSerializer<String>
                             } else {
                                 final LdapAttribute attr = new LdapAttribute(e.getKey());
                                 for (final JsonValue v : (JsonArray) e.getValue()) {
-                                    attr.addStringValue(((JsonString) v).getString());
+                                    attr.addStringValues(((JsonString) v).getString());
                                 }
-                                entry.addAttribute(attr);
+                                entry.addAttributes(attr);
                             }
                         }
                     }
diff --git a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/DefaultAuthenticationResultSerializerTest.java b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/DefaultAuthenticationResultSerializerTest.java
index 89bc91726..08c700c16 100644
--- a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/DefaultAuthenticationResultSerializerTest.java
+++ b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/DefaultAuthenticationResultSerializerTest.java
@@ -58,7 +58,6 @@ import net.shibboleth.shared.spring.resource.ResourceHelper;
 
 import org.ldaptive.LdapAttribute;
 import org.ldaptive.LdapEntry;
-import org.ldaptive.SortBehavior;
 import org.ldaptive.jaas.LdapPrincipal;
 import org.opensaml.profile.context.ProfileRequestContext;
 import org.opensaml.profile.testing.RequestContextBuilder;
@@ -339,17 +338,17 @@ public class DefaultAuthenticationResultSerializerTest {
         flowDescriptor.initialize();
         
         final AuthenticationResult result = createResult(flowDescriptor, new Subject());
-        final LdapEntry entry = new LdapEntry(SortBehavior.SORTED);
+        final LdapEntry entry = new LdapEntry();
         entry.setDn("uid=1234,ou=people,dc=shibboleth,dc=net");
-        final LdapAttribute givenName = new LdapAttribute(SortBehavior.SORTED);
+        final LdapAttribute givenName = new LdapAttribute();
         givenName.setName("givenName");
-        givenName.addStringValue("Bob", "Robert");
-        entry.addAttribute(
+        givenName.addStringValues("Bob", "Robert");
+        entry.addAttributes(
                 new LdapAttribute("cn", "Bob Cobb"),
                 givenName,
                 new LdapAttribute("sn", "Cobb"),
                 new LdapAttribute("mail", "bob at shibboleth.net"));
-        result.getSubject().getPrincipals().add(new LdapPrincipal("bob", entry));
+        result.getSubject().getPrincipals().add(new LdapPrincipal("bob", LdapEntry.sort(entry)));
 
         final ProfileRequestContext prc = getProfileRequestContext(Collections.singletonList(flowDescriptor));
         assertTrue(result.getReuseCondition().test(prc));
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 ed8e0d4e7..332bb5046 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
@@ -34,7 +34,7 @@ import org.ldaptive.auth.AuthenticationResponse;
 import org.ldaptive.auth.AuthenticationResponseHandler;
 import org.ldaptive.auth.AuthenticationResultCode;
 import org.ldaptive.auth.Authenticator;
-import org.ldaptive.auth.BindAuthenticationHandler;
+import org.ldaptive.auth.SimpleBindAuthenticationHandler;
 import org.ldaptive.auth.SearchDnResolver;
 import org.ldaptive.auth.ext.PasswordPolicyAccountState;
 import org.ldaptive.control.PasswordPolicyControl;
@@ -81,7 +81,7 @@ public class LDAPCredentialValidatorTest extends BaseAuthenticationContextTest {
 
     private TemplateSearchDnResolver dnResolver;
 
-    private BindAuthenticationHandler authHandler;
+    private SimpleBindAuthenticationHandler authHandler;
 
     private Authenticator authenticator;
 
@@ -109,7 +109,7 @@ public class LDAPCredentialValidatorTest extends BaseAuthenticationContextTest {
                 VelocityEngine.newVelocityEngine(), "(uid=$usernamePasswordContext.username)");
         dnResolver.setBaseDn("ou=people,dc=shibboleth,dc=net");
 
-        authHandler = new BindAuthenticationHandler(new DefaultConnectionFactory("ldap://localhost:10389"));
+        authHandler = new SimpleBindAuthenticationHandler(new DefaultConnectionFactory("ldap://localhost:10389"));
 
         authenticator = new Authenticator(dnResolver, authHandler);
     }
@@ -174,6 +174,7 @@ public class LDAPCredentialValidatorTest extends BaseAuthenticationContextTest {
 
         final Event event = action.execute(src);
         Assert.assertNull(ac.getAuthenticationResult());
+        Assert.assertNull(ac.getSubcontext(LDAPResponseContext.class));
         Assert.assertNull(ac.getSubcontext(AuthenticationErrorContext.class));
         ActionTestingSupport.assertEvent(event, AuthnEventIds.NO_CREDENTIALS);
     }
@@ -234,7 +235,7 @@ public class LDAPCredentialValidatorTest extends BaseAuthenticationContextTest {
         ac.setAttemptedFlow(authenticationFlows.get(0));
         
         validator.setAuthenticator(new Authenticator(dnResolver,
-                new BindAuthenticationHandler(new DefaultConnectionFactory("ldap://unknown:389"))));
+                new SimpleBindAuthenticationHandler(new DefaultConnectionFactory("ldap://unknown:389"))));
         validator.initialize();
         
         action.initialize();
@@ -244,15 +245,10 @@ public class LDAPCredentialValidatorTest extends BaseAuthenticationContextTest {
         final Event event = action.execute(src);
 
         Assert.assertNull(ac.getAuthenticationResult());
-        LDAPResponseContext lrc = ac.getSubcontext(LDAPResponseContext.class);
-        Assert.assertNotNull(lrc.getAuthenticationResponse());
-        Assert.assertEquals(lrc.getAuthenticationResponse().getAuthenticationResultCode(),
-                AuthenticationResultCode.AUTHENTICATION_HANDLER_FAILURE);
-
+        Assert.assertNull(ac.getSubcontext(LDAPResponseContext.class));
         AuthenticationErrorContext aec = ac.getSubcontext(AuthenticationErrorContext.class);
         Assert.assertNotNull(aec);
         ActionTestingSupport.assertEvent(event, AuthnEventIds.AUTHN_EXCEPTION);
-        System.err.println("EXCEPTIONS:: " + aec.getExceptions());
         Assert.assertEquals(aec.getExceptions().size(), 1);
         Assert.assertEquals(aec.getClassifiedErrors().size(), 0);
     }
@@ -301,6 +297,7 @@ public class LDAPCredentialValidatorTest extends BaseAuthenticationContextTest {
 
         final Event event = action.execute(src);
         Assert.assertNull(ac.getAuthenticationResult());
+        Assert.assertNull(ac.getSubcontext(LDAPResponseContext.class));
         Assert.assertNull(ac.getSubcontext(AuthenticationErrorContext.class));
         ActionTestingSupport.assertEvent(event, AuthnEventIds.INVALID_CREDENTIALS);
     }
@@ -341,11 +338,9 @@ public class LDAPCredentialValidatorTest extends BaseAuthenticationContextTest {
         ac.setAttemptedFlow(authenticationFlows.get(0));
 
         Authenticator errorAuthenticator = new Authenticator(dnResolver, authHandler);
-        errorAuthenticator.setAuthenticationResponseHandlers(new AuthenticationResponseHandler() {
-            public void handle(AuthenticationResponse response) throws LdapException {
-                response.setAccountState(new PasswordPolicyAccountState(PasswordPolicyControl.Error.PASSWORD_EXPIRED));
-            }
-        });
+        errorAuthenticator.setResponseHandlers(
+            response -> response.setAccountState(
+                new PasswordPolicyAccountState(PasswordPolicyControl.Error.PASSWORD_EXPIRED)));
         validator.setAuthenticator(errorAuthenticator);
         validator.initialize();
         
@@ -376,12 +371,9 @@ public class LDAPCredentialValidatorTest extends BaseAuthenticationContextTest {
         ac.setAttemptedFlow(authenticationFlows.get(0));
 
         Authenticator errorAuthenticator = new Authenticator(dnResolver, authHandler);
-        errorAuthenticator.setAuthenticationResponseHandlers(new AuthenticationResponseHandler() {
-            public void handle(AuthenticationResponse response) throws LdapException {
-                response.setAccountState(
-                        new PasswordPolicyAccountState(PasswordPolicyControl.Error.CHANGE_AFTER_RESET));
-            }
-        });
+        errorAuthenticator.setResponseHandlers(
+            response -> response.setAccountState(
+                new PasswordPolicyAccountState(PasswordPolicyControl.Error.CHANGE_AFTER_RESET)));
         validator.setAuthenticator(errorAuthenticator);
         validator.initialize();
         
@@ -422,12 +414,9 @@ public class LDAPCredentialValidatorTest extends BaseAuthenticationContextTest {
         ac.setAttemptedFlow(authenticationFlows.get(0));
 
         Authenticator warningAuthenticator = new Authenticator(dnResolver, authHandler);
-        warningAuthenticator.setAuthenticationResponseHandlers(new AuthenticationResponseHandler() {
-            public void handle(AuthenticationResponse response) throws LdapException {
-                response.setAccountState(
-                        new AccountState(new AccountState.DefaultWarning(ZonedDateTime.now(), 10)));
-            }
-        });
+        warningAuthenticator.setResponseHandlers(
+            response -> response.setAccountState(
+                new AccountState(new AccountState.DefaultWarning(ZonedDateTime.now(), 10))));
         validator.setAuthenticator(warningAuthenticator);
         validator.initialize();
         
diff --git a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateCredentialsTest.java b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateCredentialsTest.java
index 8080c2a47..7a83949e9 100644
--- a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateCredentialsTest.java
+++ b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/impl/ValidateCredentialsTest.java
@@ -28,7 +28,7 @@ import java.util.function.Supplier;
 import org.ldaptive.DefaultConnectionFactory;
 import org.ldaptive.auth.AuthenticationResultCode;
 import org.ldaptive.auth.Authenticator;
-import org.ldaptive.auth.BindAuthenticationHandler;
+import org.ldaptive.auth.SimpleBindAuthenticationHandler;
 import org.ldaptive.jaas.LdapPrincipal;
 import org.springframework.core.io.FileSystemResource;
 import org.springframework.mock.web.MockHttpServletRequest;
@@ -69,7 +69,7 @@ public class ValidateCredentialsTest extends BaseAuthenticationContextTest {
 
     private TemplateSearchDnResolver dnResolver;
 
-    private BindAuthenticationHandler authHandler;
+    private SimpleBindAuthenticationHandler authHandler;
 
     private Authenticator authenticator;
 
@@ -97,7 +97,7 @@ public class ValidateCredentialsTest extends BaseAuthenticationContextTest {
                 VelocityEngine.newVelocityEngine(), "(uid=$usernamePasswordContext.username)");
         dnResolver.setBaseDn("ou=people,dc=shibboleth,dc=net");
 
-        authHandler = new BindAuthenticationHandler(new DefaultConnectionFactory("ldap://localhost:10389"));
+        authHandler = new SimpleBindAuthenticationHandler(new DefaultConnectionFactory("ldap://localhost:10389"));
 
         authenticator = new Authenticator(dnResolver, authHandler);
     }

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


More information about the commits mailing list