AMBARI-21307 Testing the attribute detection - fixing bugs, cleanup
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/953a98fc Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/953a98fc Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/953a98fc Branch: refs/heads/feature-branch-AMBARI-21307 Commit: 953a98fc8e12ed9d7dc90296f52087e52bb58311 Parents: 5c3c6e9 Author: lpuskas <lpus...@apache.org> Authored: Tue Sep 12 18:58:44 2017 +0200 Committer: lpuskas <lpus...@apache.org> Committed: Thu Oct 12 19:25:50 2017 +0200 ---------------------------------------------------------------------- .../api/services/ldap/LdapConfigurationService.java | 11 +++++++---- .../ambari/server/ldap/AmbariLdapConfiguration.java | 2 +- .../org/apache/ambari/server/ldap/LdapModule.java | 3 +++ .../ambari/server/ldap/service/AmbariLdapFacade.java | 6 ++++++ .../ads/DefaultAttributeDetectionService.java | 15 +++++++-------- .../service/ads/DefaultLdapConfigurationService.java | 3 --- .../ads/detectors/GroupMemberAttrDetector.java | 3 +++ .../service/ads/detectors/GroupNameAttrDetector.java | 3 +++ .../ads/detectors/GroupObjectClassDetector.java | 3 +++ .../ads/detectors/UserGroupMemberAttrDetector.java | 3 +++ .../service/ads/detectors/UserNameAttrDetector.java | 3 +++ .../ads/DefaultAttributeDetectionServiceTest.java | 3 +-- .../ads/DefaultLdapConfigurationServiceTest.java | 4 ++-- 13 files changed, 42 insertions(+), 20 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/953a98fc/ambari-server/src/main/java/org/apache/ambari/server/api/services/ldap/LdapConfigurationService.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/api/services/ldap/LdapConfigurationService.java b/ambari-server/src/main/java/org/apache/ambari/server/api/services/ldap/LdapConfigurationService.java index 66809c3..ae47a87 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/api/services/ldap/LdapConfigurationService.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/api/services/ldap/LdapConfigurationService.java @@ -86,6 +86,7 @@ public class LdapConfigurationService extends AmbariConfigurationService { authorize(); Set<String> groups = Sets.newHashSet(); + Object responseEntity = null; Result result = new ResultImpl(new ResultStatus(ResultStatus.STATUS.OK)); try { @@ -108,13 +109,14 @@ public class LdapConfigurationService extends AmbariConfigurationService { LOGGER.info("Testing LDAP attributes ...."); groups = ldapFacade.checkLdapAttributes(ldapConfigurationRequest.getRequestInfo().getParameters(), ambariLdapConfiguration); - setResult(groups, result); + responseEntity = groups; break; case DETECT_ATTRIBUTES: LOGGER.info("Detecting LDAP attributes ..."); - ldapFacade.detectAttributes(ambariLdapConfiguration); + ambariLdapConfiguration = ldapFacade.detectAttributes(ambariLdapConfiguration); + responseEntity = ambariLdapConfiguration; break; default: @@ -123,10 +125,11 @@ public class LdapConfigurationService extends AmbariConfigurationService { } } catch (Exception e) { - result.setResultStatus(new ResultStatus(ResultStatus.STATUS.BAD_REQUEST, e)); + result.setResultStatus(new ResultStatus(ResultStatus.STATUS.SERVER_ERROR, e)); + responseEntity = e.getMessage(); } - return Response.status(result.getStatus().getStatusCode()).entity(getResultSerializer().serialize(result)).build(); + return Response.status(result.getStatus().getStatusCode()).entity(responseEntity).build(); } private void setResult(Set<String> groups, Result result) { http://git-wip-us.apache.org/repos/asf/ambari/blob/953a98fc/ambari-server/src/main/java/org/apache/ambari/server/ldap/AmbariLdapConfiguration.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/ldap/AmbariLdapConfiguration.java b/ambari-server/src/main/java/org/apache/ambari/server/ldap/AmbariLdapConfiguration.java index ebb567d..5bdda7a 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/ldap/AmbariLdapConfiguration.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/ldap/AmbariLdapConfiguration.java @@ -62,7 +62,7 @@ public class AmbariLdapConfiguration { GROUP_OBJECT_CLASS("ambari.ldap.attributes.group.object_class"), GROUP_NAME_ATTRIBUTE("ambari.ldap.attributes.group.name_attr"), GROUP_MEMBER_ATTRIBUTE("ambari.ldap.attributes.group.member_attr"), - GROUP_SEARCH_BASE("ambari.ldap.attributes.user.search_base"), + GROUP_SEARCH_BASE("ambari.ldap.attributes.group.search_base"), USER_SEARCH_FILTER("ambari.ldap.advanced.user_search_filter"), USER_MEMBER_REPLACE_PATTERN("ambari.ldap.advanced.user_member_replace_pattern"), http://git-wip-us.apache.org/repos/asf/ambari/blob/953a98fc/ambari-server/src/main/java/org/apache/ambari/server/ldap/LdapModule.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/ldap/LdapModule.java b/ambari-server/src/main/java/org/apache/ambari/server/ldap/LdapModule.java index 81f2a44..5d6a698 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/ldap/LdapModule.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/ldap/LdapModule.java @@ -17,8 +17,10 @@ package org.apache.ambari.server.ldap; import org.apache.ambari.server.ldap.service.AmbariLdapConfigurationProvider; import org.apache.ambari.server.ldap.service.AmbariLdapFacade; +import org.apache.ambari.server.ldap.service.LdapAttributeDetectionService; import org.apache.ambari.server.ldap.service.LdapConnectionService; import org.apache.ambari.server.ldap.service.LdapFacade; +import org.apache.ambari.server.ldap.service.ads.DefaultAttributeDetectionService; import org.apache.ambari.server.ldap.service.ads.DefaultLdapConfigurationService; import org.apache.ambari.server.ldap.service.ads.DefaultLdapConnectionService; @@ -35,6 +37,7 @@ public class LdapModule extends AbstractModule { bind(LdapFacade.class).to(AmbariLdapFacade.class); bind(LdapConfigurationService.class).to(DefaultLdapConfigurationService.class); bind(LdapConnectionService.class).to(DefaultLdapConnectionService.class); + bind(LdapAttributeDetectionService.class).to(DefaultAttributeDetectionService.class); bind(AmbariLdapConfiguration.class).toProvider(AmbariLdapConfigurationProvider.class); http://git-wip-us.apache.org/repos/asf/ambari/blob/953a98fc/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 d2bdef3..719bb7b 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 @@ -95,13 +95,18 @@ public class AmbariLdapFacade implements LdapFacade { LdapConnection connection = ldapConnectionService.getBoundLdapConnection(ambariLdapConfiguration); try { + // decorate the configuration with detected user attributes ambariLdapConfiguration = ldapAttributeDetectionService.detectLdapUserAttributes(connection, ambariLdapConfiguration); + + // decorate the configuration with detected group attributes 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(); @@ -126,6 +131,7 @@ public class AmbariLdapFacade implements LdapFacade { LOGGER.info("Testing LDAP user attributes with test user: {}", userName); String userDn = ldapConfigurationService.checkUserAttributes(ldapConnection, userName, testUserPass, ldapConfiguration); + // todo handle the case where group membership is stored in the user rather than the group LOGGER.info("Testing LDAP group attributes with test user dn: {}", userDn); Set<String> groups = ldapConfigurationService.checkGroupAttributes(ldapConnection, userDn, ldapConfiguration); http://git-wip-us.apache.org/repos/asf/ambari/blob/953a98fc/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultAttributeDetectionService.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultAttributeDetectionService.java b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultAttributeDetectionService.java index b3a4fde..e5254b5 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultAttributeDetectionService.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultAttributeDetectionService.java @@ -48,21 +48,22 @@ public class DefaultAttributeDetectionService implements LdapAttributeDetectionS @Inject - private UserNameAttrDetector userNameAttrDetector = new UserNameAttrDetector(); // todo remove instantition + private UserNameAttrDetector userNameAttrDetector; @Inject - private UserObjectClassDetector userObjectClassDetector = new UserObjectClassDetector(); // todo remove instantition + private UserObjectClassDetector userObjectClassDetector; @Inject - private UserGroupMemberAttrDetector userGroupMemberAttrDetector = new UserGroupMemberAttrDetector(); // todo remove instantition + private UserGroupMemberAttrDetector userGroupMemberAttrDetector; @Inject - private GroupNameAttrDetector groupNameAttrDetector = new GroupNameAttrDetector(); // todo remove instantition + private GroupNameAttrDetector groupNameAttrDetector; @Inject - private GroupObjectClassDetector groupObjectClassDetector = new GroupObjectClassDetector(); // todo remove instantition + private GroupObjectClassDetector groupObjectClassDetector; - private GroupMemberAttrDetector groupMemberAttrDetector = new GroupMemberAttrDetector(); // todo remove instantition + @Inject + private GroupMemberAttrDetector groupMemberAttrDetector; @Inject public DefaultAttributeDetectionService() { @@ -81,8 +82,6 @@ public class DefaultAttributeDetectionService implements LdapAttributeDetectionS SearchCursor searchCursor = null; try { - // todo should the bind operation be done in the facade? - connection.bind(ambariLdapConfiguration.bindDn(), ambariLdapConfiguration.bindPassword()); SearchRequest searchRequest = assembleUserSearchRequest(ambariLdapConfiguration); http://git-wip-us.apache.org/repos/asf/ambari/blob/953a98fc/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 5735d7d..c90b5ac 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 @@ -49,9 +49,6 @@ public class DefaultLdapConfigurationService implements LdapConfigurationService private static final Logger LOGGER = LoggerFactory.getLogger(DefaultLdapConfigurationService.class); - /** - * Facilitating the instantiation - */ @Inject public DefaultLdapConfigurationService() { } http://git-wip-us.apache.org/repos/asf/ambari/blob/953a98fc/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/GroupMemberAttrDetector.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/GroupMemberAttrDetector.java b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/GroupMemberAttrDetector.java index 6931736..aa444ab 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/GroupMemberAttrDetector.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/GroupMemberAttrDetector.java @@ -14,6 +14,8 @@ package org.apache.ambari.server.ldap.service.ads.detectors; +import javax.inject.Inject; + import org.apache.directory.api.ldap.model.entry.Entry; public class GroupMemberAttrDetector extends OccurranceAndWeightBasedDetector { @@ -42,6 +44,7 @@ public class GroupMemberAttrDetector extends OccurranceAndWeightBasedDetector { } + @Inject public GroupMemberAttrDetector() { for (GroupMemberAttr groupMemberAttr : GroupMemberAttr.values()) { occurranceMap().put(groupMemberAttr.attrName(), 0); http://git-wip-us.apache.org/repos/asf/ambari/blob/953a98fc/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/GroupNameAttrDetector.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/GroupNameAttrDetector.java b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/GroupNameAttrDetector.java index f868383..d4dcdff 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/GroupNameAttrDetector.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/GroupNameAttrDetector.java @@ -14,6 +14,8 @@ package org.apache.ambari.server.ldap.service.ads.detectors; +import javax.inject.Inject; + import org.apache.directory.api.ldap.model.entry.Entry; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -45,6 +47,7 @@ public class GroupNameAttrDetector extends OccurranceAndWeightBasedDetector { } + @Inject public GroupNameAttrDetector() { for (GroupNameAttr groupNameAttr : GroupNameAttr.values()) { http://git-wip-us.apache.org/repos/asf/ambari/blob/953a98fc/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/GroupObjectClassDetector.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/GroupObjectClassDetector.java b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/GroupObjectClassDetector.java index fddc5a5..88824c4 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/GroupObjectClassDetector.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/GroupObjectClassDetector.java @@ -14,6 +14,8 @@ package org.apache.ambari.server.ldap.service.ads.detectors; +import javax.inject.Inject; + import org.apache.directory.api.ldap.model.entry.Entry; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -50,6 +52,7 @@ public class GroupObjectClassDetector extends OccurranceAndWeightBasedDetector { } + @Inject public GroupObjectClassDetector() { for (ObjectClassValue ocVal : ObjectClassValue.values()) { occurranceMap().put(ocVal.ocVal(), 0); http://git-wip-us.apache.org/repos/asf/ambari/blob/953a98fc/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/UserGroupMemberAttrDetector.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/UserGroupMemberAttrDetector.java b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/UserGroupMemberAttrDetector.java index c3f2ab4..913c2b6 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/UserGroupMemberAttrDetector.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/UserGroupMemberAttrDetector.java @@ -14,6 +14,8 @@ package org.apache.ambari.server.ldap.service.ads.detectors; +import javax.inject.Inject; + import org.apache.directory.api.ldap.model.entry.Entry; public class UserGroupMemberAttrDetector extends OccurranceAndWeightBasedDetector { @@ -42,6 +44,7 @@ public class UserGroupMemberAttrDetector extends OccurranceAndWeightBasedDetecto } + @Inject public UserGroupMemberAttrDetector() { for (UserGroupMemberAttr userGroupMemberAttr : UserGroupMemberAttr.values()) { occurranceMap().put(userGroupMemberAttr.attrName(), 0); http://git-wip-us.apache.org/repos/asf/ambari/blob/953a98fc/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/UserNameAttrDetector.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/UserNameAttrDetector.java b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/UserNameAttrDetector.java index 40bf09b..eade3c8 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/UserNameAttrDetector.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/UserNameAttrDetector.java @@ -14,6 +14,8 @@ package org.apache.ambari.server.ldap.service.ads.detectors; +import javax.inject.Inject; + import org.apache.directory.api.ldap.model.entry.Entry; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -44,6 +46,7 @@ public class UserNameAttrDetector extends OccurranceAndWeightBasedDetector { } + @Inject public UserNameAttrDetector() { for (UserNameAttrs nameAttr : UserNameAttrs.values()) { occurranceMap().put(nameAttr.attrName(), 0); http://git-wip-us.apache.org/repos/asf/ambari/blob/953a98fc/ambari-server/src/test/java/org/apache/ambari/server/ldap/service/ads/DefaultAttributeDetectionServiceTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/ldap/service/ads/DefaultAttributeDetectionServiceTest.java b/ambari-server/src/test/java/org/apache/ambari/server/ldap/service/ads/DefaultAttributeDetectionServiceTest.java index 08f2d6c..9b03b86 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/ldap/service/ads/DefaultAttributeDetectionServiceTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/ldap/service/ads/DefaultAttributeDetectionServiceTest.java @@ -21,7 +21,6 @@ import org.apache.ambari.server.ldap.AmbariLdapConfiguration; import org.apache.ambari.server.ldap.service.LdapConnectionService; import org.apache.directory.api.ldap.model.constants.SchemaConstants; import org.apache.directory.ldap.client.api.LdapConnection; -import org.apache.directory.ldap.client.api.LdapNetworkConnection; import org.easymock.EasyMockRule; import org.easymock.TestSubject; import org.junit.Assert; @@ -76,7 +75,7 @@ public class DefaultAttributeDetectionServiceTest { // GIVEN AmbariLdapConfiguration ambariLdapConfiguration = new AmbariLdapConfiguration(getTestPropertiesMap()); LdapConnectionService connectionService = new DefaultLdapConnectionService(); - LdapNetworkConnection ldapConnection = connectionService.createLdapConnection(ambariLdapConfiguration); + LdapConnection ldapConnection = connectionService.createLdapConnection(ambariLdapConfiguration); // WHEN AmbariLdapConfiguration config = attributeDetectionService.detectLdapUserAttributes(ldapConnection, ambariLdapConfiguration); http://git-wip-us.apache.org/repos/asf/ambari/blob/953a98fc/ambari-server/src/test/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConfigurationServiceTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConfigurationServiceTest.java b/ambari-server/src/test/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConfigurationServiceTest.java index e023c6c..1e69012 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConfigurationServiceTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConfigurationServiceTest.java @@ -86,7 +86,7 @@ public class DefaultLdapConfigurationServiceTest { AmbariLdapConfiguration ambariLdapConfiguration = new AmbariLdapConfiguration(ldapPropsMap); LdapConnectionService connectionService = new DefaultLdapConnectionService(); - LdapNetworkConnection ldapConnection = connectionService.createLdapConnection(ambariLdapConfiguration); + LdapConnection ldapConnection = connectionService.createLdapConnection(ambariLdapConfiguration); ldapConfigurationService.checkUserAttributes(ldapConnection, "einstein", "", ambariLdapConfiguration); } @@ -108,7 +108,7 @@ public class DefaultLdapConfigurationServiceTest { AmbariLdapConfiguration ambariLdapConfiguration = new AmbariLdapConfiguration(ldapPropsMap); LdapConnectionService connectionService = new DefaultLdapConnectionService(); - LdapNetworkConnection ldapConnection = connectionService.createLdapConnection(ambariLdapConfiguration); + LdapConnection ldapConnection = connectionService.createLdapConnection(ambariLdapConfiguration); ldapConfigurationService.checkGroupAttributes(ldapConnection, "uid=einstein,dc=example,dc=com", ambariLdapConfiguration); }
