You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by lp...@apache.org on 2017/10/19 12:45:04 UTC
[30/46] ambari git commit: AMBARI-2130 ldap connections handled in
thefacade. Code cleanup
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/01295fe1
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/01295fe1
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/01295fe1
Branch: refs/heads/feature-branch-AMBARI-21307
Commit: 01295fe17d9b2f4017e113ec1014065cc7cff545
Parents: 92f2cc5
Author: lpuskas <lp...@apache.org>
Authored: Tue Sep 12 15:38:25 2017 +0200
Committer: lpuskas <lp...@apache.org>
Committed: Thu Oct 19 14:42:00 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/01295fe1/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/01295fe1/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/01295fe1/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/01295fe1/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/01295fe1/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/01295fe1/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)) {