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 <[email protected]> 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: [email protected] For additional commands, e-mail: [email protected]
