Repository: ambari Updated Branches: refs/heads/branch-2.1 0d5cb02de -> aa283e6a9
AMBARI-12176 - LDAP sync needs to distinguish group vs user membership (tbeerbower) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/aa283e6a Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/aa283e6a Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/aa283e6a Branch: refs/heads/branch-2.1 Commit: aa283e6a9041171210f7c1b9b222fc07b99c954c Parents: 0d5cb02 Author: tbeerbower <tbeerbo...@hortonworks.com> Authored: Fri Jun 26 21:07:33 2015 -0400 Committer: tbeerbower <tbeerbo...@hortonworks.com> Committed: Fri Jun 26 21:08:24 2015 -0400 ---------------------------------------------------------------------- .../security/ldap/AmbariLdapDataPopulator.java | 60 +++++++++++++------- .../ldap/AmbariLdapDataPopulatorTest.java | 29 +++++----- 2 files changed, 53 insertions(+), 36 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/aa283e6a/ambari-server/src/main/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulator.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulator.java b/ambari-server/src/main/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulator.java index ada4171..1d8fca1 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulator.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulator.java @@ -184,7 +184,7 @@ public class AmbariLdapDataPopulator { } else { batchInfo.getGroupsToBeCreated().add(groupName); } - refreshGroupMembers(batchInfo, groupDto, internalUsersMap); + refreshGroupMembers(batchInfo, groupDto, internalUsersMap, null); } for (Entry<String, Group> internalGroup : internalGroupsMap.entrySet()) { if (internalGroup.getValue().isLdapGroup()) { @@ -258,7 +258,7 @@ public class AmbariLdapDataPopulator { } else { batchInfo.getGroupsToBeCreated().add(groupName); } - refreshGroupMembers(batchInfo, groupDto, internalUsersMap); + refreshGroupMembers(batchInfo, groupDto, internalUsersMap, null); } return batchInfo; @@ -316,7 +316,7 @@ public class AmbariLdapDataPopulator { batchInfo.getGroupsToBeRemoved().add(group.getGroupName()); } else { LdapGroupDto groupDto = groupDtos.iterator().next(); - refreshGroupMembers(batchInfo, groupDto, internalUsersMap); + refreshGroupMembers(batchInfo, groupDto, internalUsersMap, null); } } } @@ -350,15 +350,31 @@ public class AmbariLdapDataPopulator { * @param batchInfo batch update object * @param group ldap group * @param internalUsers map of internal users + * @param groupMemberAttributes set of group member attributes that have already been refreshed * @throws AmbariException if group refresh failed */ - protected void refreshGroupMembers(LdapBatchDto batchInfo, LdapGroupDto group, Map<String, User> internalUsers) + protected void refreshGroupMembers(LdapBatchDto batchInfo, LdapGroupDto group, Map<String, User> internalUsers, Set<String> groupMemberAttributes) throws AmbariException { Set<String> externalMembers = new HashSet<String>(); + + if (groupMemberAttributes == null) { + groupMemberAttributes = new HashSet<String>(); + } + for (String memberAttributeValue: group.getMemberAttributes()) { LdapUserDto groupMember = getLdapUserByMemberAttr(memberAttributeValue); if (groupMember != null) { externalMembers.add(groupMember.getUserName()); + } else { + // if we haven't already processed this group + if (!groupMemberAttributes.contains(memberAttributeValue)) { + // if the member is another group then add all of its members + LdapGroupDto subGroup = getLdapGroupByMemberAttr(memberAttributeValue); + if (subGroup != null) { + groupMemberAttributes.add(memberAttributeValue); + refreshGroupMembers(batchInfo, subGroup, internalUsers, groupMemberAttributes); + } + } } } String groupName = group.getGroupName(); @@ -419,22 +435,33 @@ public class AmbariLdapDataPopulator { } /** - * Get the LDAP member for the given member attribute. + * Get the LDAP user member for the given member attribute. * * @param memberAttributeValue the member attribute value * * @return the user for the given member attribute; null if not found */ protected LdapUserDto getLdapUserByMemberAttr(String memberAttributeValue) { - LdapUserDto dto = getLdapUser(memberAttributeValue); - if (dto == null) { - Set<LdapUserDto> filteredLdapUsers = getFilteredLdapUsers( - new EqualsFilter(OBJECT_CLASS_ATTRIBUTE, ldapServerProperties.getUserObjectClass()), - getMemberFilter(memberAttributeValue)); + Set<LdapUserDto> filteredLdapUsers = getFilteredLdapUsers( + new EqualsFilter(OBJECT_CLASS_ATTRIBUTE, ldapServerProperties.getUserObjectClass()), + getMemberFilter(memberAttributeValue)); - dto = (filteredLdapUsers.isEmpty()) ? null : filteredLdapUsers.iterator().next(); - } - return dto; + return (filteredLdapUsers.isEmpty()) ? null : filteredLdapUsers.iterator().next(); + } + + /** + * Get the LDAP group member for the given member attribute. + * + * @param memberAttributeValue the member attribute value + * + * @return the group for the given member attribute; null if not found + */ + protected LdapGroupDto getLdapGroupByMemberAttr(String memberAttributeValue) { + Set<LdapGroupDto> filteredLdapUsers = getFilteredLdapGroups( + new EqualsFilter(OBJECT_CLASS_ATTRIBUTE, ldapServerProperties.getGroupObjectClass()), + getMemberFilter(memberAttributeValue)); + + return (filteredLdapUsers.isEmpty()) ? null : filteredLdapUsers.iterator().next(); } /** @@ -499,13 +526,6 @@ public class AmbariLdapDataPopulator { return getFilteredLdapUsers(userObjectFilter); } - // get the user for the given distinguished name; null if not found - private LdapUserDto getLdapUser(String distinguishedName) { - final LdapTemplate ldapTemplate = loadLdapTemplate(); - - return (LdapUserDto) ldapTemplate.lookup(distinguishedName, new LdapUserContextMapper(ldapServerProperties)); - } - private Set<LdapUserDto> getFilteredLdapUsers(Filter...filters) { AndFilter andFilter = new AndFilter(); for (Filter filter : filters) { http://git-wip-us.apache.org/repos/asf/ambari/blob/aa283e6a/ambari-server/src/test/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulatorTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulatorTest.java b/ambari-server/src/test/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulatorTest.java index 4968730..fba56f9 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulatorTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulatorTest.java @@ -42,6 +42,7 @@ import org.apache.ambari.server.security.authorization.Users; import org.easymock.Capture; import org.easymock.EasyMock; import org.easymock.IAnswer; +import org.junit.Ignore; import org.junit.Test; import org.springframework.ldap.control.PagedResultsCookie; import org.springframework.ldap.control.PagedResultsDirContextProcessor; @@ -249,7 +250,7 @@ public class AmbariLdapDataPopulatorTest { expect(populator.getLdapGroups("group2")).andReturn(Collections.EMPTY_SET); LdapGroupDto externalGroup1 = createNiceMock(LdapGroupDto.class); LdapBatchDto batchInfo = new LdapBatchDto(); - populator.refreshGroupMembers(eq(batchInfo), eq(externalGroup1), anyObject(Map.class)); + populator.refreshGroupMembers(eq(batchInfo), eq(externalGroup1), anyObject(Map.class), anyObject(Set.class)); expectLastCall(); expect(populator.getLdapGroups("group4")).andReturn(Collections.singleton(externalGroup1)); expect(populator.getLdapGroups("group5")).andReturn(Collections.EMPTY_SET); @@ -320,12 +321,12 @@ public class AmbariLdapDataPopulatorTest { LdapBatchDto batchInfo = new LdapBatchDto(); Set<LdapGroupDto> externalGroups = createSet(externalGroup3, externalGroup4); for (LdapGroupDto externalGroup : externalGroups) { - populator.refreshGroupMembers(eq(batchInfo), eq(externalGroup), anyObject(Map.class)); + populator.refreshGroupMembers(eq(batchInfo), eq(externalGroup), anyObject(Map.class), anyObject(Set.class)); expectLastCall(); } - populator.refreshGroupMembers(eq(batchInfo), eq(externalGroup1), anyObject(Map.class)); + populator.refreshGroupMembers(eq(batchInfo), eq(externalGroup1), anyObject(Map.class), anyObject(Set.class)); expectLastCall(); - populator.refreshGroupMembers(eq(batchInfo), eq(externalGroup2), anyObject(Map.class)); + populator.refreshGroupMembers(eq(batchInfo), eq(externalGroup2), anyObject(Map.class), anyObject(Set.class)); expectLastCall(); expect(populator.getLdapGroups("x*")).andReturn(externalGroups); expect(populator.getLdapGroups("group1")).andReturn(Collections.singleton(externalGroup1)); @@ -399,10 +400,10 @@ public class AmbariLdapDataPopulatorTest { LdapBatchDto batchInfo = new LdapBatchDto(); Set<LdapGroupDto> externalGroups = createSet(externalGroup3, externalGroup4); for (LdapGroupDto externalGroup : externalGroups) { - populator.refreshGroupMembers(eq(batchInfo), eq(externalGroup), anyObject(Map.class)); + populator.refreshGroupMembers(eq(batchInfo), eq(externalGroup), anyObject(Map.class), anyObject(Set.class)); expectLastCall(); } - populator.refreshGroupMembers(eq(batchInfo), eq(externalGroup2), anyObject(Map.class)); + populator.refreshGroupMembers(eq(batchInfo), eq(externalGroup2), anyObject(Map.class), anyObject(Set.class)); expectLastCall(); expect(populator.getLdapGroups("x*")).andReturn(externalGroups); expect(populator.getLdapGroups("group2")).andReturn(Collections.singleton(externalGroup2)); @@ -473,7 +474,7 @@ public class AmbariLdapDataPopulatorTest { LdapBatchDto batchInfo = new LdapBatchDto(); Set<LdapGroupDto> externalGroups = createSet(externalGroup1, externalGroup2, externalGroup3, externalGroup4); for (LdapGroupDto externalGroup : externalGroups) { - populator.refreshGroupMembers(eq(batchInfo), eq(externalGroup), anyObject(Map.class)); + populator.refreshGroupMembers(eq(batchInfo), eq(externalGroup), anyObject(Map.class), anyObject(Set.class)); expectLastCall(); } expect(populator.getLdapGroups("group*")).andReturn(externalGroups); @@ -603,7 +604,7 @@ public class AmbariLdapDataPopulatorTest { LdapBatchDto batchInfo = new LdapBatchDto(); Set<LdapGroupDto> externalGroups = createSet(externalGroup1, externalGroup2, externalGroup3, externalGroup4); for (LdapGroupDto externalGroup : externalGroups) { - populator.refreshGroupMembers(eq(batchInfo), eq(externalGroup), anyObject(Map.class)); + populator.refreshGroupMembers(eq(batchInfo), eq(externalGroup), anyObject(Map.class), anyObject(Set.class)); expectLastCall(); } @@ -664,7 +665,7 @@ public class AmbariLdapDataPopulatorTest { LdapBatchDto batchInfo = new LdapBatchDto(); Set<LdapGroupDto> externalGroups = createSet(externalGroup1, externalGroup2); for (LdapGroupDto externalGroup : externalGroups) { - populator.refreshGroupMembers(eq(batchInfo), eq(externalGroup), anyObject(Map.class)); + populator.refreshGroupMembers(eq(batchInfo), eq(externalGroup), anyObject(Map.class), anyObject(Set.class)); expectLastCall(); } expect(populator.getExternalLdapGroupInfo()).andReturn(externalGroups); @@ -728,7 +729,7 @@ public class AmbariLdapDataPopulatorTest { LdapBatchDto batchInfo = new LdapBatchDto(); Set<LdapGroupDto> externalGroups = createSet(externalGroup1); for (LdapGroupDto externalGroup : externalGroups) { - populator.refreshGroupMembers(eq(batchInfo), eq(externalGroup), anyObject(Map.class)); + populator.refreshGroupMembers(eq(batchInfo), eq(externalGroup), anyObject(Map.class), anyObject(Set.class)); expectLastCall(); } expect(populator.getExternalLdapGroupInfo()).andReturn(externalGroups); @@ -791,7 +792,7 @@ public class AmbariLdapDataPopulatorTest { LdapBatchDto batchInfo = new LdapBatchDto(); Set<LdapGroupDto> externalGroups = createSet(externalGroup1, externalGroup2); for (LdapGroupDto externalGroup : externalGroups) { - populator.refreshGroupMembers(eq(batchInfo), eq(externalGroup), anyObject(Map.class)); + populator.refreshGroupMembers(eq(batchInfo), eq(externalGroup), anyObject(Map.class), anyObject(Set.class)); expectLastCall(); } expect(populator.getExternalLdapGroupInfo()).andReturn(externalGroups); @@ -1392,7 +1393,7 @@ public class AmbariLdapDataPopulatorTest { internalUsers.putAll(internalMembers); internalUsers.put("user2", user2); - populator.refreshGroupMembers(batchInfo, externalGroup, internalUsers); + populator.refreshGroupMembers(batchInfo, externalGroup, internalUsers, null); Set<String> groupMembersToAdd = new HashSet<String>(); for (LdapUserGroupMemberDto ldapUserGroupMemberDto : batchInfo.getMembershipToAdd()) { @@ -1498,9 +1499,6 @@ public class AmbariLdapDataPopulatorTest { expect(processor.getCookie()).andReturn(cookie).anyTimes(); expect(cookie.getCookie()).andReturn(null).anyTimes(); - expect(ldapTemplate.lookup(eq("uid=foo,dc=example,dc=com"), capture(contextMapperCapture))).andReturn(dto); - - expect(ldapTemplate.lookup(eq("foo"), capture(contextMapperCapture))).andReturn(null); expect(ldapTemplate.search(eq("baseDN"), eq("(&(objectClass=objectClass)(|(dn=foo)(uid=foo)))"), anyObject(SearchControls.class), capture(contextMapperCapture), eq(processor))).andReturn(list); replay(ldapTemplate, ldapServerProperties, users, configuration, processor, cookie); @@ -1510,7 +1508,6 @@ public class AmbariLdapDataPopulatorTest { populator.setLdapTemplate(ldapTemplate); populator.setProcessor(processor); - assertEquals(dto, populator.getLdapUserByMemberAttr("uid=foo,dc=example,dc=com")); assertEquals(dto, populator.getLdapUserByMemberAttr("foo")); verify(ldapTemplate, ldapServerProperties, users, configuration, processor, cookie);