AMBARI-2130 ldap connections handled in thefacade. Code cleanup
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/5c3c6e91 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/5c3c6e91 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/5c3c6e91 Branch: refs/heads/feature-branch-AMBARI-21307 Commit: 5c3c6e913051219ef42679a5376ccd38c44b5abf Parents: 19827f8 Author: lpuskas <lpus...@apache.org> Authored: Tue Sep 12 15:38:25 2017 +0200 Committer: lpuskas <lpus...@apache.org> Committed: Thu Oct 12 19:25:50 2017 +0200 ---------------------------------------------------------------------- .../server/ldap/service/AmbariLdapFacade.java | 51 +++++++++---- .../ldap/service/LdapConnectionService.java | 12 ++- .../ambari/server/ldap/service/LdapFacade.java | 2 +- .../ads/DefaultLdapConfigurationService.java | 77 ++++---------------- .../ads/DefaultLdapConnectionService.java | 41 ++++++++++- .../OccurranceAndWeightBasedDetector.java | 2 +- 6 files changed, 103 insertions(+), 82 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/5c3c6e91/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/AmbariLdapFacade.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/AmbariLdapFacade.java b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/AmbariLdapFacade.java index f159418..d2bdef3 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/AmbariLdapFacade.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/AmbariLdapFacade.java @@ -19,7 +19,6 @@ import java.util.Map; import java.util.Set; import javax.inject.Inject; -import javax.inject.Provider; import javax.inject.Singleton; import org.apache.ambari.server.ldap.AmbariLdapConfiguration; @@ -58,35 +57,59 @@ public class AmbariLdapFacade implements LdapFacade { @Inject private LdapAttributeDetectionService ldapAttributeDetectionService; - //todo remove this, added for testing purposes only - @Inject - private Provider<AmbariLdapConfiguration> ambariLdapConfigurationProvider; - @Inject public AmbariLdapFacade() { } @Override public void checkConnection(AmbariLdapConfiguration ambariLdapConfiguration) throws AmbariLdapException { + LdapConnection connection = null; try { + LOGGER.info("Validating LDAP connection related configuration based on: {}", ambariLdapConfiguration); - LdapConnection connection = ldapConnectionService.createLdapConnection(ambariLdapConfiguration); + connection = ldapConnectionService.getBoundLdapConnection(ambariLdapConfiguration); ldapConfigurationService.checkConnection(connection, ambariLdapConfiguration); - } catch (AmbariLdapException e) { + LOGGER.info("Validating LDAP connection related configuration: SUCCESS"); + + } catch (Exception e) { + LOGGER.error("Validating LDAP connection configuration failed", e); - throw e; + throw new AmbariLdapException(e); + + } finally { + try { + connection.unBind(); + connection.close(); + } catch (Exception e) { + throw new AmbariLdapException(e); + } } - LOGGER.info("Validating LDAP connection related configuration: SUCCESS"); + } @Override - public AmbariLdapConfiguration detectAttributes(AmbariLdapConfiguration ambariLdapConfiguration) { + public AmbariLdapConfiguration detectAttributes(AmbariLdapConfiguration ambariLdapConfiguration) throws AmbariLdapException { LOGGER.info("Detecting LDAP configuration attributes ..."); - LdapConnection connection = ldapConnectionService.createLdapConnection(ambariLdapConfiguration); - ambariLdapConfiguration = ldapAttributeDetectionService.detectLdapUserAttributes(connection, ambariLdapConfiguration); - return ambariLdapConfiguration; + LdapConnection connection = ldapConnectionService.getBoundLdapConnection(ambariLdapConfiguration); + try { + + ambariLdapConfiguration = ldapAttributeDetectionService.detectLdapUserAttributes(connection, ambariLdapConfiguration); + ambariLdapConfiguration = ldapAttributeDetectionService.detectLdapGroupAttributes(connection, ambariLdapConfiguration); + return ambariLdapConfiguration; + + } catch (Exception e) { + LOGGER.error("Error during LDAP attribute detection", e); + throw new AmbariLdapException(e); + } finally { + try { + connection.unBind(); + connection.close(); + } catch (Exception e) { + throw new AmbariLdapException(e); + } + } } @Override @@ -98,7 +121,7 @@ public class AmbariLdapFacade implements LdapFacade { throw new IllegalArgumentException("No test user available for testing LDAP attributes"); } - LdapConnection ldapConnection = ldapConnectionService.createLdapConnection(ldapConfiguration); + LdapConnection ldapConnection = ldapConnectionService.getBoundLdapConnection(ldapConfiguration); LOGGER.info("Testing LDAP user attributes with test user: {}", userName); String userDn = ldapConfigurationService.checkUserAttributes(ldapConnection, userName, testUserPass, ldapConfiguration); http://git-wip-us.apache.org/repos/asf/ambari/blob/5c3c6e91/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/LdapConnectionService.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/LdapConnectionService.java b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/LdapConnectionService.java index 50ee8ed..b4daeaa 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/LdapConnectionService.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/LdapConnectionService.java @@ -15,7 +15,7 @@ package org.apache.ambari.server.ldap.service; import org.apache.ambari.server.ldap.AmbariLdapConfiguration; -import org.apache.directory.ldap.client.api.LdapNetworkConnection; +import org.apache.directory.ldap.client.api.LdapConnection; /** * Contract defining factory methods for creating LDAP connection instances. @@ -29,7 +29,15 @@ public interface LdapConnectionService { * @param ambariLdapConfiguration configuration instance with information for creating the connection instance * @return a set up LdapConnection instance */ - LdapNetworkConnection createLdapConnection(AmbariLdapConfiguration ambariLdapConfiguration); + LdapConnection createLdapConnection(AmbariLdapConfiguration ambariLdapConfiguration); + + /** + * Creates an LdapConnection instance and binds to the LDAP server based on the provided configuration entries + * + * @param ambariLdapConfiguration ambari configuration instance + * @return + */ + LdapConnection getBoundLdapConnection(AmbariLdapConfiguration ambariLdapConfiguration); } http://git-wip-us.apache.org/repos/asf/ambari/blob/5c3c6e91/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/LdapFacade.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/LdapFacade.java b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/LdapFacade.java index 7cd25da..6060d7f 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/LdapFacade.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/LdapFacade.java @@ -39,7 +39,7 @@ public interface LdapFacade { * * @param ambariLdapConfiguration */ - AmbariLdapConfiguration detectAttributes(AmbariLdapConfiguration ambariLdapConfiguration); + AmbariLdapConfiguration detectAttributes(AmbariLdapConfiguration ambariLdapConfiguration) throws AmbariLdapException; /** * Checks user and group related LDAP configuration attributes in the configuration object with the help of the provided parameters http://git-wip-us.apache.org/repos/asf/ambari/blob/5c3c6e91/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConfigurationService.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConfigurationService.java b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConfigurationService.java index fa2e44b..5735d7d 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConfigurationService.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConfigurationService.java @@ -14,24 +14,20 @@ package org.apache.ambari.server.ldap.service.ads; -import java.io.IOException; import java.util.List; import java.util.Set; import javax.inject.Inject; import javax.inject.Singleton; -import org.apache.ambari.server.AmbariException; import org.apache.ambari.server.ldap.AmbariLdapConfiguration; import org.apache.ambari.server.ldap.LdapConfigurationService; import org.apache.ambari.server.ldap.service.AmbariLdapException; -import org.apache.ambari.server.ldap.service.LdapConnectionService; import org.apache.directory.api.ldap.codec.decorators.SearchResultEntryDecorator; import org.apache.directory.api.ldap.model.constants.SchemaConstants; import org.apache.directory.api.ldap.model.cursor.EntryCursor; import org.apache.directory.api.ldap.model.cursor.SearchCursor; import org.apache.directory.api.ldap.model.entry.Entry; -import org.apache.directory.api.ldap.model.exception.LdapException; import org.apache.directory.api.ldap.model.message.Response; import org.apache.directory.api.ldap.model.message.SearchRequest; import org.apache.directory.api.ldap.model.message.SearchRequestImpl; @@ -53,9 +49,6 @@ public class DefaultLdapConfigurationService implements LdapConfigurationService private static final Logger LOGGER = LoggerFactory.getLogger(DefaultLdapConfigurationService.class); - @Inject - private LdapConnectionService ldapConnectionService; - /** * Facilitating the instantiation */ @@ -65,12 +58,12 @@ public class DefaultLdapConfigurationService implements LdapConfigurationService @Override public void checkConnection(LdapConnection ldapConnection, AmbariLdapConfiguration ambariLdapConfiguration) throws AmbariLdapException { - try { - bind(ambariLdapConfiguration, ldapConnection); - } catch (LdapException e) { - LOGGER.error("Could not connect to the LDAP server", e); - throw new AmbariLdapException(e); + + if (!ldapConnection.isConnected()) { + LOGGER.error("Could not connect to the LDAP server"); + throw new AmbariLdapException("Could not connect to the LDAP server. Configuration: " + ambariLdapConfiguration); } + } @@ -80,22 +73,20 @@ public class DefaultLdapConfigurationService implements LdapConfigurationService * * Invalid attributes are signaled by throwing an exception. * + * @param ldapConnection connection instance used to connect to the LDAP server * @param testUserName the test username * @param testPassword the test password * @param ambariLdapConfiguration configuration instance holding ldap configuration details * @return the DN of the test user - * @throws AmbariException if the attributes are not valid or any errors occurs + * @throws AmbariLdapException if an error occurs */ @Override public String checkUserAttributes(LdapConnection ldapConnection, String testUserName, String testPassword, AmbariLdapConfiguration ambariLdapConfiguration) throws AmbariLdapException { - SearchCursor searchCursor = null; String userDn = null; + EntryCursor entryCursor = null; try { LOGGER.info("Checking user attributes for user {} r ...", testUserName); - // bind anonimously or with manager data - bind(ambariLdapConfiguration, ldapConnection); - // set up a filter based on the provided attributes String filter = FilterBuilder.and( FilterBuilder.equal(SchemaConstants.OBJECT_CLASS_AT, ambariLdapConfiguration.userObjectClass()), @@ -103,7 +94,7 @@ public class DefaultLdapConfigurationService implements LdapConfigurationService .toString(); LOGGER.info("Searching for the user: {} using the search filter: {}", testUserName, filter); - EntryCursor entryCursor = ldapConnection.search(new Dn(ambariLdapConfiguration.userSearchBase()), filter, SearchScope.SUBTREE); + entryCursor = ldapConnection.search(new Dn(ambariLdapConfiguration.userSearchBase()), filter, SearchScope.SUBTREE); // collecting search result entries List<Entry> users = Lists.newArrayList(); @@ -127,7 +118,9 @@ public class DefaultLdapConfigurationService implements LdapConfigurationService throw new AmbariLdapException(e.getMessage(), e); } finally { - closeResources(ldapConnection, searchCursor); + if (null != entryCursor) { + entryCursor.close(); + } } return userDn; } @@ -141,8 +134,6 @@ public class DefaultLdapConfigurationService implements LdapConfigurationService try { LOGGER.info("Checking group attributes for user dn {} ...", userDn); - bind(ambariLdapConfiguration, ldapConnection); - // set up a filter based on the provided attributes String filter = FilterBuilder.and( FilterBuilder.equal(SchemaConstants.OBJECT_CLASS_AT, ambariLdapConfiguration.groupObjectClass()), @@ -171,36 +162,14 @@ public class DefaultLdapConfigurationService implements LdapConfigurationService throw new AmbariLdapException(e.getMessage(), e); } finally { - closeResources(ldapConnection, searchCursor); + if (null != searchCursor) { + searchCursor.close(); + } } return processGroupResults(groupResponses, ambariLdapConfiguration); } - /** - * Binds to the LDAP server (anonimously or wit manager credentials) - * - * @param ambariLdapConfiguration configuration instance - * @param connection connection instance - * @throws LdapException if the bind operation fails - */ - private void bind(AmbariLdapConfiguration ambariLdapConfiguration, LdapConnection connection) throws LdapException { - LOGGER.info("Connecting to LDAP ...."); - if (!ambariLdapConfiguration.anonymousBind()) { - LOGGER.debug("Anonimous binding not supported, binding with the manager detailas..."); - connection.bind(ambariLdapConfiguration.bindDn(), ambariLdapConfiguration.bindPassword()); - } else { - LOGGER.debug("Binding anonymously ..."); - connection.bind(); - } - - if (!connection.isConnected()) { - LOGGER.error("Not connected to the LDAP server. Connection instance: {}", connection); - throw new IllegalStateException("The connection to the LDAP server is not alive"); - } - LOGGER.info("Connected to LDAP."); - } - /** * Extracts meaningful values from the search result. @@ -220,22 +189,6 @@ public class DefaultLdapConfigurationService implements LdapConfigurationService return groupStrSet; } - private void closeResources(LdapConnection connection, SearchCursor searchCursor) { - LOGGER.debug("Housekeeping: closing the connection and the search cursor ..."); - - if (null != searchCursor) { - // this method is idempotent - searchCursor.close(); - } - - if (null != connection) { - try { - connection.close(); - } catch (IOException e) { - LOGGER.error("Exception occurred while closing the connection", e); - } - } - } } http://git-wip-us.apache.org/repos/asf/ambari/blob/5c3c6e91/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConnectionService.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConnectionService.java b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConnectionService.java index f39df54..457e23e 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConnectionService.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConnectionService.java @@ -32,6 +32,7 @@ import javax.inject.Singleton; import org.apache.ambari.server.ldap.AmbariLdapConfiguration; import org.apache.ambari.server.ldap.service.LdapConnectionService; +import org.apache.directory.ldap.client.api.LdapConnection; import org.apache.directory.ldap.client.api.LdapConnectionConfig; import org.apache.directory.ldap.client.api.LdapNetworkConnection; import org.slf4j.Logger; @@ -45,18 +46,54 @@ public class DefaultLdapConnectionService implements LdapConnectionService { @Override public LdapNetworkConnection createLdapConnection(AmbariLdapConfiguration ambariLdapConfiguration) { LOGGER.debug("Creating ldap connection instance from: {}", ambariLdapConfiguration); + return new LdapNetworkConnection(getLdapConnectionConfig(ambariLdapConfiguration)); } + @Override + public LdapConnection getBoundLdapConnection(AmbariLdapConfiguration ambariLdapConfiguration) { + LOGGER.info("Creating LDAP connection instance and binding to LDAP server ..."); + + try { + LdapConnection connection = createLdapConnection(ambariLdapConfiguration); + + if (!ambariLdapConfiguration.anonymousBind()) { + + LOGGER.debug("Anonymous binding not supported, binding with the manager detailas..."); + connection.bind(ambariLdapConfiguration.bindDn(), ambariLdapConfiguration.bindPassword()); + + } else { + + LOGGER.debug("Binding anonymously ..."); + connection.bind(); + + } + + if (!connection.isConnected()) { + + LOGGER.error("Not connected to the LDAP server. Connection instance: {}", connection); + throw new IllegalStateException("The connection to the LDAP server is not alive"); + + } + + LOGGER.info("Connected / bound to LDAP server."); + return connection; + + } catch (Exception e) { + LOGGER.error("Could not create or bind LdapConnection", e); + throw new IllegalArgumentException(e); + } + + } + private LdapConnectionConfig getLdapConnectionConfig(AmbariLdapConfiguration ambariAmbariLdapConfiguration) { - LOGGER.debug("Creating a configuration instance based on the ambari configuration: {}", ambariAmbariLdapConfiguration); + LOGGER.debug("Creating a LDAP connection config instance based on the ambari configuration: {}", ambariAmbariLdapConfiguration); LdapConnectionConfig ldapConnectionConfig = new LdapConnectionConfig(); ldapConnectionConfig.setLdapHost(ambariAmbariLdapConfiguration.serverHost()); ldapConnectionConfig.setLdapPort(ambariAmbariLdapConfiguration.serverPort()); ldapConnectionConfig.setUseSsl(ambariAmbariLdapConfiguration.useSSL()); - // todo set the other values as required return ldapConnectionConfig; } http://git-wip-us.apache.org/repos/asf/ambari/blob/5c3c6e91/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/OccurranceAndWeightBasedDetector.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/OccurranceAndWeightBasedDetector.java b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/OccurranceAndWeightBasedDetector.java index 8aaf6c1..71dfb42 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/OccurranceAndWeightBasedDetector.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/OccurranceAndWeightBasedDetector.java @@ -78,7 +78,7 @@ public abstract class OccurranceAndWeightBasedDetector implements AttributeDetec @Override public void collect(Entry entry) { - LOGGER.info("Collecting ldap attributes/values form entry with dn: [{]]", entry.getDn()); + LOGGER.info("Collecting ldap attributes/values form entry with dn: [{}]", entry.getDn()); for (String attributeValue : occurranceMap().keySet()) { if (applies(entry, attributeValue)) {
