AMBARI-6920. Admin Views: Sync LDAP does not sync the LDAP groups. (mahadev)

ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PrivilegeResourceProvider.java

@@ -342,6 +342,9 @@ public abstract class PrivilegeResourceProvider<T> extends AbstractResourceProvi
       }
     } else if (PrincipalTypeEntity.USER_PRINCIPAL_TYPE_NAME.equalsIgnoreCase(principalType)) {
       UserEntity userEntity = userDAO.findLocalUserByName(principalName);
+      if (userEntity == null) {
+        userEntity = userDAO.findLdapUserByName(principalName);
+      }
       if (userEntity != null) {
         entity.setPrincipal(principalDAO.findById(userEntity.getPrincipal().getId()));
       }

ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariLdapDataPopulator.java

@@ -65,7 +65,7 @@ public class AmbariLdapDataPopulator {
   /**
    * LDAP specific properties.
    */
-  private LdapServerProperties ldapServerProperties;
+  protected LdapServerProperties ldapServerProperties;
 
   /**
    * LDAP template for making search queries.
@@ -86,14 +86,7 @@ public class AmbariLdapDataPopulator {
   public boolean isLdapEnabled() {
     try {
       final LdapTemplate ldapTemplate = loadLdapTemplate();
-      ldapTemplate.search(ldapServerProperties.getBaseDN(),
-          "(objectclass=person)", new AttributesMapper() {
-
-            public Object mapFromAttributes(Attributes attributes)
-                throws NamingException {
-              return "";
-            }
-          });
+      ldapTemplate.list(ldapServerProperties.getBaseDN());
       return true;
     } catch (Exception ex) {
       LOG.error("Could not connect to LDAP server", ex);
@@ -217,7 +210,7 @@ public class AmbariLdapDataPopulator {
    * @param groupName group name
    * @throws AmbariException if group refresh failed
    */
-  private void refreshGroupMembers(String groupName) throws AmbariException {
+  protected void refreshGroupMembers(String groupName) throws AmbariException {
     final Set<String> externalMembers = getExternalLdapGroupMembers(groupName);
     final Map<String, User> internalUsers = getInternalUsers();
     final Map<String, User> internalMembers = getInternalMembers(groupName);
@@ -233,8 +226,9 @@ public class AmbariLdapDataPopulator {
         internalMembers.remove(externalMember);
         internalUsers.remove(externalMember);
       } else {
-        users.createUser(externalMember, "", true, false);
+        users.createUser(externalMember, "");
         users.setUserLdap(externalMember);
+        users.addMemberToGroup(groupName, externalMember);
       }
     }
     for (Entry<String, User> userToBeUnsynced: internalMembers.entrySet()) {
@@ -248,7 +242,7 @@ public class AmbariLdapDataPopulator {
    *
    * @throws AmbariException
    */
-  private void cleanUpLdapUsersWithoutGroup() throws AmbariException {
+  protected void cleanUpLdapUsersWithoutGroup() throws AmbariException {
     final List<User> allUsers = users.getAllUsers();
     for (User user: allUsers) {
       if (user.isLdapUser() && user.getGroups().isEmpty()) {
@@ -264,7 +258,7 @@ public class AmbariLdapDataPopulator {
    *
    * @return set of user names
    */
-  private Set<String> getExternalLdapGroupNames() {
+  protected Set<String> getExternalLdapGroupNames() {
     final Set<String> groups = new HashSet<String>();
     final LdapTemplate ldapTemplate = loadLdapTemplate();
     final EqualsFilter equalsFilter = new EqualsFilter("objectClass",
@@ -287,7 +281,7 @@ public class AmbariLdapDataPopulator {
    *
    * @return set of user names
    */
-  private Set<String> getExternalLdapUserNames() {
+  protected Set<String> getExternalLdapUserNames() {
     final Set<String> users = new HashSet<String>();
     final LdapTemplate ldapTemplate = loadLdapTemplate();
     final EqualsFilter equalsFilter = new EqualsFilter("objectClass",
@@ -311,7 +305,7 @@ public class AmbariLdapDataPopulator {
    * @param groupName group name
    * @return set of group names
    */
-  private Set<String> getExternalLdapGroupMembers(String groupName) {
+  protected Set<String> getExternalLdapGroupMembers(String groupName) {
     final Set<String> members = new HashSet<String>();
     final LdapTemplate ldapTemplate = loadLdapTemplate();
     final AndFilter andFilter = new AndFilter();
@@ -337,7 +331,7 @@ public class AmbariLdapDataPopulator {
    *
    * @return map of GroupName-Group pairs
    */
-  private Map<String, Group> getInternalGroups() {
+  protected Map<String, Group> getInternalGroups() {
     final List<Group> internalGroups = users.getAllGroups();
     final Map<String, Group> internalGroupsMap = new HashMap<String, Group>();
     for (Group group : internalGroups) {
@@ -351,7 +345,7 @@ public class AmbariLdapDataPopulator {
    *
    * @return map of UserName-User pairs
    */
-  private Map<String, User> getInternalUsers() {
+  protected Map<String, User> getInternalUsers() {
     final List<User> internalUsers = users.getAllUsers();
     final Map<String, User> internalUsersMap = new HashMap<String, User>();
     for (User user : internalUsers) {
@@ -366,7 +360,7 @@ public class AmbariLdapDataPopulator {
    * @param groupName group name
    * @return map of UserName-User pairs
    */
-  private Map<String, User> getInternalMembers(String groupName) {
+  protected Map<String, User> getInternalMembers(String groupName) {
     final Collection<User> internalMembers = users.getGroupMembers(groupName);
     final Map<String, User> internalMembersMap = new HashMap<String, User>();
     for (User user : internalMembers) {
@@ -380,7 +374,7 @@ public class AmbariLdapDataPopulator {
    *
    * @return LdapTemplate instance
    */
-  private LdapTemplate loadLdapTemplate() {
+  protected LdapTemplate loadLdapTemplate() {
     final LdapServerProperties properties = configuration
         .getLdapServerProperties();
     if (ldapTemplate == null || !properties.equals(ldapServerProperties)) {

ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLdapDataPopulatorTest.java

@@ -0,0 +1,261 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ambari.server.security.authorization;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import junit.framework.Assert;
+
+import org.apache.ambari.server.AmbariException;
+import org.apache.ambari.server.configuration.Configuration;
+import org.apache.ambari.server.orm.entities.GroupEntity;
+import org.apache.ambari.server.orm.entities.MemberEntity;
+import org.apache.ambari.server.orm.entities.PrincipalEntity;
+import org.apache.ambari.server.orm.entities.PrivilegeEntity;
+import org.apache.ambari.server.orm.entities.RoleEntity;
+import org.apache.ambari.server.orm.entities.UserEntity;
+import org.easymock.Capture;
+import org.easymock.EasyMock;
+import org.easymock.IAnswer;
+import org.junit.Test;
+import org.springframework.ldap.core.LdapTemplate;
+
+public class AmbariLdapDataPopulatorTest {
+  private static class AmbariLdapDataPopulatorTestInstance extends AmbariLdapDataPopulator {
+
+    public AmbariLdapDataPopulatorTestInstance(Configuration configuration,
+        Users users) {
+      super(configuration, users);
+      this.ldapServerProperties = EasyMock.createNiceMock(LdapServerProperties.class);
+    }
+
+    final LdapTemplate ldapTemplate = EasyMock.createNiceMock(LdapTemplate.class);
+
+    @Override
+    protected LdapTemplate loadLdapTemplate() {
+      return ldapTemplate;
+    }
+
+    public LdapServerProperties getLdapServerProperties() {
+      return this.ldapServerProperties;
+    }
+  }
+
+  @Test
+  public void testRefreshGroupMembers() throws AmbariException {
+    final Configuration configuration = EasyMock.createNiceMock(Configuration.class);
+    final Users users = EasyMock.createNiceMock(Users.class);
+
+    final GroupEntity ldapGroup = new GroupEntity();
+    ldapGroup.setGroupId(1);
+    ldapGroup.setGroupName("ldapGroup");
+    ldapGroup.setLdapGroup(true);
+    ldapGroup.setMemberEntities(new HashSet<MemberEntity>());
+
+    final User ldapUserWithoutGroup = createLdapUserWithoutGroup();
+    final User ldapUserWithGroup = createLdapUserWithGroup(ldapGroup);
+    final User localUserWithoutGroup = createLocalUserWithoutGroup();
+    final User localUserWithGroup = createLocalUserWithGroup(ldapGroup);
+
+    final AmbariLdapDataPopulator populator = new AmbariLdapDataPopulatorTestInstance(configuration, users) {
+      @Override
+      protected Set<String> getExternalLdapGroupMembers(String groupName) {
+        return new HashSet<String>() {
+          {
+            add(ldapUserWithGroup.getUserName());
+            add(ldapUserWithoutGroup.getUserName());
+          }
+        };
+      }
+
+      @Override
+      protected Map<String, User> getInternalUsers() {
+        return new HashMap<String, User>() {
+          {
+            put(localUserWithGroup.getUserName(), localUserWithGroup);
+            put(localUserWithoutGroup.getUserName(), localUserWithoutGroup);
+          }
+        };
+      }
+
+      @Override
+      protected Map<String, User> getInternalMembers(String groupName) {
+        return new HashMap<String, User>() {
+          {
+            put(localUserWithGroup.getUserName(), localUserWithGroup);
+          }
+        };
+      }
+    };
+
+    users.createUser(EasyMock.<String> anyObject(), EasyMock.<String> anyObject());
+    EasyMock.expectLastCall().times(2);
+
+    users.addMemberToGroup(EasyMock.<String> anyObject(), EasyMock.<String> anyObject());
+    EasyMock.expectLastCall().times(2);
+
+    EasyMock.replay(users);
+
+    populator.refreshGroupMembers(ldapGroup.getGroupName());
+
+    EasyMock.verify(users);
+  }
+
+  @Test
+  public void testIsLdapEnabled() {
+    final Configuration configuration = EasyMock.createNiceMock(Configuration.class);
+    final Users users = EasyMock.createNiceMock(Users.class);
+
+    final AmbariLdapDataPopulator populator = new AmbariLdapDataPopulatorTestInstance(configuration, users);
+
+    EasyMock.expect(populator.loadLdapTemplate().list(EasyMock. <String>anyObject())).andReturn(Collections.emptyList()).once();
+    EasyMock.replay(populator.loadLdapTemplate());
+
+    populator.isLdapEnabled();
+    EasyMock.verify(populator.loadLdapTemplate());
+  }
+
+  @Test
+  public void testIsLdapEnabled_reallyEnabled() {
+    final Configuration configuration = EasyMock.createNiceMock(Configuration.class);
+    final Users users = EasyMock.createNiceMock(Users.class);
+
+    final AmbariLdapDataPopulator populator = new AmbariLdapDataPopulatorTestInstance(configuration, users);
+
+    EasyMock.expect(populator.loadLdapTemplate().list(EasyMock. <String>anyObject())).andReturn(Collections.emptyList()).once();
+    EasyMock.replay(populator.loadLdapTemplate());
+
+    Assert.assertTrue(populator.isLdapEnabled());
+    EasyMock.verify(populator.loadLdapTemplate());
+  }
+
+  @Test
+  public void testIsLdapEnabled_reallyDisabled() {
+    final Configuration configuration = EasyMock.createNiceMock(Configuration.class);
+    final Users users = EasyMock.createNiceMock(Users.class);
+
+    final AmbariLdapDataPopulator populator = new AmbariLdapDataPopulatorTestInstance(configuration, users);
+
+    EasyMock.expect(populator.loadLdapTemplate().list(EasyMock. <String>anyObject())).andThrow(new NullPointerException()).once();
+    EasyMock.replay(populator.loadLdapTemplate());
+
+    Assert.assertFalse(populator.isLdapEnabled());
+    EasyMock.verify(populator.loadLdapTemplate());
+  }
+
+  @Test
+  @SuppressWarnings("serial")
+  public void testCleanUpLdapUsersWithoutGroup() throws AmbariException {
+    final Configuration configuration = EasyMock.createNiceMock(Configuration.class);
+    final Users users = EasyMock.createNiceMock(Users.class);
+
+    final GroupEntity ldapGroup = new GroupEntity();
+    ldapGroup.setGroupId(1);
+    ldapGroup.setGroupName("ldapGroup");
+    ldapGroup.setLdapGroup(true);
+    ldapGroup.setMemberEntities(new HashSet<MemberEntity>());
+
+    final User ldapUserWithoutGroup = createLdapUserWithoutGroup();
+    final User ldapUserWithGroup = createLdapUserWithGroup(ldapGroup);
+    final User localUserWithoutGroup = createLocalUserWithoutGroup();
+    final User localUserWithGroup = createLocalUserWithGroup(ldapGroup);
+
+    final List<User> allUsers = new ArrayList<User>() {
+      {
+        add(ldapUserWithoutGroup);
+        add(ldapUserWithGroup);
+        add(localUserWithoutGroup);
+        add(localUserWithGroup);
+      }
+    };
+    EasyMock.expect(users.getAllUsers()).andReturn(new ArrayList<User>(allUsers));
+
+    final List<User> removedUsers = new ArrayList<User>();
+    final Capture<User> userCapture = new Capture<User>();
+    users.removeUser(EasyMock.capture(userCapture));
+    EasyMock.expectLastCall().andAnswer(new IAnswer<Void>() {
+      @Override
+      public Void answer() throws Throwable {
+        removedUsers.add(userCapture.getValue());
+        allUsers.remove(userCapture.getValue());
+        return null;
+      }
+    });
+
+    EasyMock.replay(users);
+
+    final AmbariLdapDataPopulator populator = new AmbariLdapDataPopulatorTestInstance(configuration, users);
+    populator.cleanUpLdapUsersWithoutGroup();
+
+    Assert.assertEquals(removedUsers.size(), 1);
+    Assert.assertEquals(allUsers.size(), 3);
+    Assert.assertTrue(allUsers.contains(ldapUserWithGroup));
+    Assert.assertTrue(allUsers.contains(localUserWithoutGroup));
+    Assert.assertTrue(allUsers.contains(localUserWithGroup));
+    Assert.assertEquals(removedUsers.get(0), ldapUserWithoutGroup);
+
+    EasyMock.verify(users);
+  }
+
+  private static int userIdCounter = 1;
+
+  private User createUser(String name, boolean ldapUser, GroupEntity group) {
+    final UserEntity userEntity = new UserEntity();
+    userEntity.setUserId(userIdCounter++);
+    userEntity.setUserName(name);
+    userEntity.setCreateTime(new Date());
+    userEntity.setLdapUser(ldapUser);
+    userEntity.setActive(true);
+    userEntity.setMemberEntities(new HashSet<MemberEntity>());
+    userEntity.setRoleEntities(new HashSet<RoleEntity>());
+    final PrincipalEntity principalEntity = new PrincipalEntity();
+    principalEntity.setPrivileges(new HashSet<PrivilegeEntity>());
+    userEntity.setPrincipal(principalEntity);
+    if (group != null) {
+      final MemberEntity member = new MemberEntity();
+      member.setUser(userEntity);
+      member.setGroup(group);
+      group.getMemberEntities().add(member);
+      userEntity.getMemberEntities().add(member);
+    }
+    return new User(userEntity);
+  }
+
+  private User createLdapUserWithoutGroup() {
+    return createUser("LdapUserWithoutGroup", true, null);
+  }
+
+  private User createLocalUserWithoutGroup() {
+    return createUser("LocalUserWithoutGroup", false, null);
+  }
+
+  private User createLdapUserWithGroup(GroupEntity group) {
+    return createUser("LdapUserWithGroup", true, group);
+  }
+
+  private User createLocalUserWithGroup(GroupEntity group) {
+    return createUser("LocalUserWithGroup", false, group);
+  }
+}