You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by bt...@apache.org on 2021/06/11 07:36:59 UTC

[james-project] 03/18: JAMES-3594 Use Reactor to implement LDAP retries

This is an automated email from the ASF dual-hosted git repository.

btellier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git

commit 9235adc8ea525bfad6f54707ea7c968a2bcdb20f
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Sun Jun 6 20:46:48 2021 +0700

    JAMES-3594 Use Reactor to implement LDAP retries
---
 .../user/ldap/LdapRepositoryConfiguration.java     | 11 +++
 .../apache/james/user/ldap/ReadOnlyLDAPUser.java   | 36 ++++++----
 .../james/user/ldap/ReadOnlyLDAPUsersDAO.java      | 78 ++++++++++++++++------
 3 files changed, 93 insertions(+), 32 deletions(-)

diff --git a/server/data/data-ldap/src/main/java/org/apache/james/user/ldap/LdapRepositoryConfiguration.java b/server/data/data-ldap/src/main/java/org/apache/james/user/ldap/LdapRepositoryConfiguration.java
index efc79ed..a87413b 100644
--- a/server/data/data-ldap/src/main/java/org/apache/james/user/ldap/LdapRepositoryConfiguration.java
+++ b/server/data/data-ldap/src/main/java/org/apache/james/user/ldap/LdapRepositoryConfiguration.java
@@ -19,6 +19,7 @@
 
 package org.apache.james.user.ldap;
 
+import java.time.Duration;
 import java.util.Objects;
 import java.util.Optional;
 
@@ -29,6 +30,10 @@ import org.apache.james.core.Username;
 
 import com.google.common.base.Preconditions;
 
+import reactor.core.scheduler.Schedulers;
+import reactor.util.retry.Retry;
+import reactor.util.retry.RetryBackoffSpec;
+
 public class LdapRepositoryConfiguration {
     public static final String SUPPORTS_VIRTUAL_HOSTING = "supportsVirtualHosting";
 
@@ -384,6 +389,12 @@ public class LdapRepositoryConfiguration {
         return administratorId;
     }
 
+    public RetryBackoffSpec retrySpec() {
+        return Retry.backoff(getMaxRetries(), Duration.ofMillis(getRetryStartInterval() * getScale()))
+            .maxBackoff(Duration.ofMillis(getRetryMaxInterval() * getScale()))
+            .scheduler(Schedulers.elastic());
+    }
+
     @Override
     public final boolean equals(Object o) {
         if (o instanceof LdapRepositoryConfiguration) {
diff --git a/server/data/data-ldap/src/main/java/org/apache/james/user/ldap/ReadOnlyLDAPUser.java b/server/data/data-ldap/src/main/java/org/apache/james/user/ldap/ReadOnlyLDAPUser.java
index 6125fb9..b185e9f 100644
--- a/server/data/data-ldap/src/main/java/org/apache/james/user/ldap/ReadOnlyLDAPUser.java
+++ b/server/data/data-ldap/src/main/java/org/apache/james/user/ldap/ReadOnlyLDAPUser.java
@@ -30,6 +30,8 @@ import com.unboundid.ldap.sdk.BindResult;
 import com.unboundid.ldap.sdk.LDAPConnectionPool;
 import com.unboundid.ldap.sdk.LDAPException;
 
+import reactor.core.publisher.Mono;
+
 /**
  * Encapsulates the details of a user as taken from an LDAP compliant directory.
  * Instances of this class are only applicable to the
@@ -68,27 +70,29 @@ public class ReadOnlyLDAPUser implements User, Serializable {
      */
     private final LDAPConnectionPool connectionPool;
 
+    private final LdapRepositoryConfiguration configuration;
+
     /**
      * Constructs an instance for the given user-details, and which will
      * authenticate against the given host.
-     *
-     * @param connectionPool
-     *            The connectionPool for the LDAP server on which the user details are held.
-     *            This is also the host against which the user will be
-     *            authenticated, when {@link #verifyPassword(String)} is
-     *            invoked.
-     * @param userName
+     *  @param userName
      *            The user-identifier/name. This is the value with which the
      *            field  will be initialised, and which will be
      *            returned by invoking {@link #getUserName()}.
      * @param userDN
      *            The distinguished (unique-key) of the user details as stored
-     *            on the LDAP directory.
+     * @param connectionPool
+ *            The connectionPool for the LDAP server on which the user details are held.
+ *            This is also the host against which the user will be
+ *            authenticated, when {@link #verifyPassword(String)} is
+ *            invoked.
+     * @param configuration
      */
-    public ReadOnlyLDAPUser(Username userName, String userDN, LDAPConnectionPool connectionPool) {
+    public ReadOnlyLDAPUser(Username userName, String userDN, LDAPConnectionPool connectionPool, LdapRepositoryConfiguration configuration) {
         this.userName = userName;
         this.userDN = userDN;
         this.connectionPool = connectionPool;
+        this.configuration = configuration;
     }
 
     /**
@@ -130,12 +134,18 @@ public class ReadOnlyLDAPUser implements User, Serializable {
     @Override
     public boolean verifyPassword(String password) {
         try {
-            BindResult bindResult = connectionPool.bindAndRevertAuthentication(userDN, password);
-            return bindResult.getResultCode()
-                .intValue() == 0;
-        } catch (LDAPException e) {
+            return Mono.fromCallable(() -> doVerifyPassword(password))
+                .retryWhen(configuration.retrySpec())
+                .block();
+        } catch (Exception e) {
             LOGGER.error("Unexpected error upon authentication", e);
             return false;
         }
     }
+
+    private boolean doVerifyPassword(String password) throws LDAPException {
+        BindResult bindResult = connectionPool.bindAndRevertAuthentication(userDN, password);
+        return bindResult.getResultCode()
+            .intValue() == 0;
+    }
 }
diff --git a/server/data/data-ldap/src/main/java/org/apache/james/user/ldap/ReadOnlyLDAPUsersDAO.java b/server/data/data-ldap/src/main/java/org/apache/james/user/ldap/ReadOnlyLDAPUsersDAO.java
index 3d406a1..0dc4ecd 100644
--- a/server/data/data-ldap/src/main/java/org/apache/james/user/ldap/ReadOnlyLDAPUsersDAO.java
+++ b/server/data/data-ldap/src/main/java/org/apache/james/user/ldap/ReadOnlyLDAPUsersDAO.java
@@ -55,6 +55,8 @@ import com.unboundid.ldap.sdk.SearchResult;
 import com.unboundid.ldap.sdk.SearchResultEntry;
 import com.unboundid.ldap.sdk.SearchScope;
 
+import reactor.core.publisher.Mono;
+
 public class ReadOnlyLDAPUsersDAO implements UsersDAO, Configurable {
     private static final Logger LOGGER = LoggerFactory.getLogger(ReadOnlyLDAPUsersDAO.class);
 
@@ -218,7 +220,7 @@ public class ReadOnlyLDAPUsersDAO implements UsersDAO, Configurable {
             if (!ldapConfiguration.getRestriction().isActivated()
                 || userInGroupsMembershipList(result.getDN(), ldapConfiguration.getRestriction().getGroupMembershipLists(connection))) {
 
-                return new ReadOnlyLDAPUser(name, result.getDN(), ldapConnectionPool);
+                return new ReadOnlyLDAPUser(name, result.getDN(), ldapConnectionPool, ldapConfiguration);
             }
             return null;
         } finally {
@@ -233,7 +235,7 @@ public class ReadOnlyLDAPUsersDAO implements UsersDAO, Configurable {
             Optional<String> userName = Optional.ofNullable(userAttributes.getAttributeValue(ldapConfiguration.getUserIdAttribute()));
             return userName
                 .map(Username::of)
-                .map(username -> new ReadOnlyLDAPUser(username, userDN, ldapConnectionPool));
+                .map(username -> new ReadOnlyLDAPUser(username, userDN, ldapConnectionPool, ldapConfiguration));
         } finally {
             ldapConnectionPool.releaseConnection(connection);
         }
@@ -241,44 +243,82 @@ public class ReadOnlyLDAPUsersDAO implements UsersDAO, Configurable {
 
     @Override
     public boolean contains(Username name) throws UsersRepositoryException {
-        return getUserByName(name).isPresent();
+        try {
+            return Mono.fromCallable(() -> doContains(name))
+                .retryWhen(ldapConfiguration.retrySpec())
+                .block();
+        } catch (Exception e) {
+            if (e.getCause() instanceof UsersRepositoryException) {
+                throw (UsersRepositoryException) e.getCause();
+            }
+            throw new UsersRepositoryException("Unable to check user existence from ldap", e);
+        }
+    }
+
+    private boolean doContains(Username name) throws LDAPException {
+        return doGetUserByName(name).isPresent();
     }
 
     @Override
     public int countUsers() throws UsersRepositoryException {
         try {
-            return Math.toIntExact(getValidUsers().stream()
-                .map(Throwing.function(this::buildUser).sneakyThrow())
-                .flatMap(Optional::stream)
-                .count());
-        } catch (LDAPException e) {
+            return Mono.fromCallable(() -> doCountUsers())
+                .retryWhen(ldapConfiguration.retrySpec())
+                .block();
+        } catch (Exception e) {
+            if (e.getCause() instanceof UsersRepositoryException) {
+                throw (UsersRepositoryException) e.getCause();
+            }
             throw new UsersRepositoryException("Unable to retrieve user count from ldap", e);
         }
     }
 
+    private int doCountUsers() throws LDAPException {
+        return Math.toIntExact(getValidUsers().stream()
+            .map(Throwing.function(this::buildUser).sneakyThrow())
+            .flatMap(Optional::stream)
+            .count());
+    }
+
     @Override
     public Optional<User> getUserByName(Username name) throws UsersRepositoryException {
         try {
-          return Optional.ofNullable(searchAndBuildUser(name));
-        } catch (LDAPException e) {
-            throw new UsersRepositoryException("Unable to retrieve user from ldap", e);
+            return Mono.fromCallable(() -> doGetUserByName(name))
+                .retryWhen(ldapConfiguration.retrySpec())
+                .block();
+        } catch (Exception e) {
+            if (e.getCause() instanceof UsersRepositoryException) {
+                throw (UsersRepositoryException) e.getCause();
+            }
+            throw new UsersRepositoryException("Unable check user existence from ldap", e);
         }
     }
 
+    private Optional<User> doGetUserByName(Username name) throws LDAPException {
+        return Optional.ofNullable(searchAndBuildUser(name));
+    }
+
     @Override
     public Iterator<Username> list() throws UsersRepositoryException {
         try {
-            return buildUserCollection(getValidUsers())
-                .stream()
-                .map(ReadOnlyLDAPUser::getUserName)
-                .iterator();
-        } catch (LDAPException namingException) {
-            throw new UsersRepositoryException(
-                    "Unable to retrieve users list from LDAP due to unknown naming error.",
-                    namingException);
+            return Mono.fromCallable(this::doList)
+                .retryWhen(ldapConfiguration.retrySpec())
+                .block();
+        } catch (Exception e) {
+            if (e.getCause() instanceof UsersRepositoryException) {
+                throw (UsersRepositoryException) e.getCause();
+            }
+            throw new UsersRepositoryException("Unable to list users from ldap", e);
         }
     }
 
+    private Iterator<Username> doList() throws LDAPException {
+        return buildUserCollection(getValidUsers())
+            .stream()
+            .map(ReadOnlyLDAPUser::getUserName)
+            .iterator();
+    }
+
     private Collection<String> getValidUsers() throws LDAPException {
         Set<String> userDNs = getAllUsersFromLDAP();
         Collection<String> validUserDNs;

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org