You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by tb...@apache.org on 2015/06/27 03:08:02 UTC

ambari git commit: AMBARI-12176 - LDAP sync needs to distinguish group vs user membership (tbeerbower)

Repository: ambari
Updated Branches:
  refs/heads/trunk f09c55528 -> ea29042ed


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/ea29042e
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/ea29042e
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/ea29042e

Branch: refs/heads/trunk
Commit: ea29042edacb9c4c9f97b0fd92b877e0327147b2
Parents: f09c555
Author: tbeerbower <tb...@hortonworks.com>
Authored: Fri Jun 26 21:07:33 2015 -0400
Committer: tbeerbower <tb...@hortonworks.com>
Committed: Fri Jun 26 21:07:47 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/ea29042e/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/ea29042e/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);