AMBARI-7714. Add unit tests related changes for UsersDAO.

ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java

@@ -723,11 +723,6 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle
         throw new AmbariException("Username and password must be supplied.");
       }
 
-      User user = users.getAnyUser(request.getUsername());
-      if (null != user) {
-        throw new AmbariException("User already exists.");
-      }
-
       users.createUser(request.getUsername(), request.getPassword(), request.isActive(), request.isAdmin(), false);
     }
   }
@@ -1643,11 +1638,11 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle
     }
     String packageList = gson.toJson(packages);
     hostParams.put(PACKAGE_LIST, packageList);
-    
+
     Set<String> userSet = configHelper.getPropertyValuesWithPropertyType(stackId, PropertyType.USER, cluster);
     String userList = gson.toJson(userSet);
     hostParams.put(USER_LIST, userList);
-    
+
     Set<String> groupSet = configHelper.getPropertyValuesWithPropertyType(stackId, PropertyType.GROUP, cluster);
     String groupList = gson.toJson(groupSet);
     hostParams.put(GROUP_LIST, groupList);
@@ -2467,7 +2462,7 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle
       }
 
       if (null != request.isActive()) {
-        users.setUserActive(u, request.isActive());
+        users.setUserActive(u.getUserName(), request.isActive());
       }
 
       if (null != request.isAdmin()) {

ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java

@@ -502,7 +502,7 @@ public class AmbariServer {
    * Creates default users if in-memory database is used
    */
   @Transactional
-  protected void initDB() {
+  protected void initDB() throws AmbariException {
     if (configs.getPersistenceType() == PersistenceType.IN_MEMORY || dbInitNeeded) {
       LOG.info("Database init needed - creating default data");
       Users users = injector.getInstance(Users.class);

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

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

ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UserDAO.java

@@ -51,10 +51,21 @@ public class UserDAO {
 
   @RequiresSession
   public List<UserEntity> findAll() {
-    TypedQuery<UserEntity> query = entityManagerProvider.get().createQuery("SELECT user FROM UserEntity user", UserEntity.class);
+    TypedQuery<UserEntity> query = entityManagerProvider.get().createQuery("SELECT user_entity FROM UserEntity user_entity", UserEntity.class);
     return daoUtils.selectList(query);
   }
 
+  @RequiresSession
+  public UserEntity findUserByName(String userName) {
+    TypedQuery<UserEntity> query = entityManagerProvider.get().createNamedQuery("userByName", UserEntity.class);
+    query.setParameter("username", userName.toLowerCase());
+    try {
+      return query.getSingleResult();
+    } catch (NoResultException e) {
+      return null;
+    }
+  }
+
   @RequiresSession
   public UserEntity findLocalUserByName(String userName) {
     TypedQuery<UserEntity> query = entityManagerProvider.get().createNamedQuery("localUserByName", UserEntity.class);
@@ -88,7 +99,7 @@ public class UserDAO {
     if (principalList == null || principalList.isEmpty()) {
       return Collections.emptyList();
     }
-    TypedQuery<UserEntity> query = entityManagerProvider.get().createQuery("SELECT user FROM UserEntity user WHERE user.principal IN :principalList", UserEntity.class);
+    TypedQuery<UserEntity> query = entityManagerProvider.get().createQuery("SELECT user_entity FROM UserEntity user_entity WHERE user_entity.principal IN :principalList", UserEntity.class);
     query.setParameter("principalList", principalList);
     return daoUtils.selectList(query);
   }

ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UserEntity.java

@@ -26,8 +26,9 @@ import java.util.Set;
 @Table(name = "users", uniqueConstraints = {@UniqueConstraint(columnNames = {"user_name", "ldap_user"})})
 @Entity
 @NamedQueries({
-    @NamedQuery(name = "localUserByName", query = "SELECT user FROM UserEntity user where lower(user.userName)=:username AND user.ldapUser=false"),
-    @NamedQuery(name = "ldapUserByName", query = "SELECT user FROM UserEntity user where lower(user.userName)=:username AND user.ldapUser=true")
+    @NamedQuery(name = "userByName", query = "SELECT user_entity from UserEntity user_entity where lower(user_entity.userName)=:username"),
+    @NamedQuery(name = "localUserByName", query = "SELECT user_entity FROM UserEntity user_entity where lower(user_entity.userName)=:username AND user_entity.ldapUser=false"),
+    @NamedQuery(name = "ldapUserByName", query = "SELECT user_entity FROM UserEntity user_entity where lower(user_entity.userName)=:username AND user_entity.ldapUser=true")
 })
 @TableGenerator(name = "user_id_generator",
     table = "ambari_sequences", pkColumnName = "sequence_name", valueColumnName = "sequence_value"

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

@@ -92,7 +92,6 @@ public class Users {
   @Inject
   private  AmbariLdapAuthenticationProvider ldapAuthenticationProvider;
 
-
   public List<User> getAllUsers() {
     List<UserEntity> userEntities = userDAO.findAll();
     List<User> users = new ArrayList<User>(userEntities.size());
@@ -104,40 +103,11 @@ public class Users {
     return users;
   }
 
-  public User getUser(int userId) throws AmbariException {
-    UserEntity userEntity = userDAO.findByPK(userId);
-    if (userEntity != null) {
-      return new User(userEntity);
-    } else {
-      throw new AmbariException("User with id '" + userId + " not found");
-    }
-  }
-
   public User getAnyUser(String userName) {
-    UserEntity userEntity = userDAO.findLdapUserByName(userName);
-    if (null == userEntity) {
-      userEntity = userDAO.findLocalUserByName(userName);
-    }
-
+    UserEntity userEntity = userDAO.findUserByName(userName);
     return (null == userEntity) ? null : new User(userEntity);
   }
 
-  public User getLocalUser(String userName) throws AmbariException{
-    UserEntity userEntity = userDAO.findLocalUserByName(userName);
-    if (userEntity == null) {
-      throw new AmbariException("User doesn't exist");
-    }
-    return new User(userEntity);
-  }
-
-  public User getLdapUser(String userName) throws AmbariException{
-    UserEntity userEntity = userDAO.findLdapUserByName(userName);
-    if (userEntity == null) {
-      throw new AmbariException("User doesn't exist");
-    }
-    return new User(userEntity);
-  }
-
   /**
    * Modifies password of local user
    * @throws AmbariException
@@ -201,15 +171,17 @@ public class Users {
 
   /**
    * Enables/disables user.
-   * @throws AmbariException
+   *
+   * @param userName user name
+   * @throws AmbariException if user does not exist
    */
-  public synchronized void setUserActive(User user, boolean active) throws AmbariException {
-    UserEntity userEntity = userDAO.findByPK(user.getUserId());
+  public synchronized void setUserActive(String userName, boolean active) throws AmbariException {
+    UserEntity userEntity = userDAO.findUserByName(userName);
     if (userEntity != null) {
       userEntity.setActive(active);
       userDAO.merge(userEntity);
     } else {
-      throw new AmbariException("User " + user + " doesn't exist");
+      throw new AmbariException("User " + userName + " doesn't exist");
     }
   }
 
@@ -220,12 +192,12 @@ public class Users {
    * @throws AmbariException if user does not exist
    */
   public synchronized void setUserLdap(String userName) throws AmbariException {
-    UserEntity userEntity = userDAO.findLocalUserByName(userName);
+    UserEntity userEntity = userDAO.findUserByName(userName);
     if (userEntity != null) {
       userEntity.setLdapUser(true);
       userDAO.merge(userEntity);
     } else {
-      throw new AmbariException("User " + userName + " doesn't exist or is already an LDAP user");
+      throw new AmbariException("User " + userName + " doesn't exist");
     }
   }
 
@@ -247,8 +219,12 @@ public class Users {
 
   /**
    * Creates new local user with provided userName and password.
+   *
+   * @param userName user name
+   * @param password password
+   * @throws AmbariException if user already exists
    */
-  public void createUser(String userName, String password) {
+  public void createUser(String userName, String password) throws AmbariException {
     createUser(userName, password, true, false, false);
   }
 
@@ -260,9 +236,14 @@ public class Users {
    * @param active is user active
    * @param admin is user admin
    * @param ldapUser is user LDAP
+   * @throws AmbariException if user already exists
    */
   @Transactional
-  public synchronized void createUser(String userName, String password, Boolean active, Boolean admin, Boolean ldapUser) {
+  public synchronized void createUser(String userName, String password, Boolean active, Boolean admin, Boolean ldapUser) throws AmbariException {
+
+    if (getAnyUser(userName) != null) {
+      throw new AmbariException("User " + userName + " already exists");
+    }
 
     // create an admin principal to represent this user
     PrincipalTypeEntity principalTypeEntity = principalTypeDAO.findById(PrincipalTypeEntity.USER_PRINCIPAL_TYPE);
@@ -420,6 +401,7 @@ public class Users {
     if (!user.getPrincipal().getPrivileges().contains(adminPrivilege)) {
       privilegeDAO.create(adminPrivilege);
       user.getPrincipal().getPrivileges().add(adminPrivilege);
+      principalDAO.merge(user.getPrincipal()); //explicit merge for Derby support
       userDAO.merge(user);
     }
   }
@@ -434,6 +416,7 @@ public class Users {
     for (PrivilegeEntity privilege: user.getPrincipal().getPrivileges()) {
       if (privilege.getPermission().getPermissionName().equals(PermissionEntity.AMBARI_ADMIN_PERMISSION_NAME)) {
         user.getPrincipal().getPrivileges().remove(privilege);
+        principalDAO.merge(user.getPrincipal()); //explicit merge for Derby support
         userDAO.merge(user);
         privilegeDAO.remove(privilege);
         break;
@@ -450,12 +433,9 @@ public class Users {
       throw new AmbariException("Group " + groupName + " doesn't exist");
     }
 
-    UserEntity userEntity = userDAO.findLocalUserByName(userName);
+    UserEntity userEntity = userDAO.findUserByName(userName);
     if (userEntity == null) {
-      userEntity = userDAO.findLdapUserByName(userName);
-      if (userEntity == null) {
-        throw new AmbariException("User " + userName + " doesn't exist");
-      }
+      throw new AmbariException("User " + userName + " doesn't exist");
     }
 
     if (isUserInGroup(userEntity, groupEntity)) {
@@ -481,12 +461,9 @@ public class Users {
       throw new AmbariException("Group " + groupName + " doesn't exist");
     }
 
-    UserEntity userEntity = userDAO.findLocalUserByName(userName);
+    UserEntity userEntity = userDAO.findUserByName(userName);
     if (userEntity == null) {
-      userEntity = userDAO.findLdapUserByName(userName);
-      if (userEntity == null) {
-        throw new AmbariException("User " + userName + " doesn't exist");
-      }
+      throw new AmbariException("User " + userName + " doesn't exist");
     }
 
     if (isUserInGroup(userEntity, groupEntity)) {
@@ -563,12 +540,9 @@ public class Users {
     // remove users
     final Set<UserEntity> usersToRemove = new HashSet<UserEntity>();
     for (String userName: batchInfo.getUsersToBeRemoved()) {
-      UserEntity userEntity = userDAO.findLocalUserByName(userName);
+      UserEntity userEntity = userDAO.findUserByName(userName);
       if (userEntity == null) {
-        userEntity = userDAO.findLdapUserByName(userName);
-        if (userEntity == null) {
-          continue;
-        }
+        continue;
       }
       allUsers.remove(userEntity.getUserName());
       usersToRemove.add(userEntity);

ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java

@@ -640,7 +640,7 @@ public class ViewRegistry {
    */
   public boolean hasPermission(PermissionEntity permissionEntity, ResourceEntity resourceEntity, String userName) {
 
-    UserEntity userEntity = userDAO.findLocalUserByName(userName);
+    UserEntity userEntity = userDAO.findUserByName(userName);
 
     if (userEntity == null) {
       return false;

ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AmbariPrivilegeResourceProviderTest.java

@@ -339,7 +339,7 @@ public class AmbariPrivilegeResourceProviderTest {
     expect(clusterDAO.findAll()).andReturn(Collections.<ClusterEntity>emptyList());
     expect(permissionDAO.findPermissionByNameAndType(EasyMock.eq("READ"), EasyMock.<ResourceTypeEntity> anyObject())).andReturn(permissionEntity);
     expect(resourceDAO.findById(EasyMock.anyLong())).andReturn(resourceEntity);
-    expect(userDAO.findLocalUserByName("admin")).andReturn(userEntity);
+    expect(userDAO.findUserByName("admin")).andReturn(userEntity);
     expect(principalDAO.findById(EasyMock.anyLong())).andReturn(principalEntity);
     expect(userEntity.getPrincipal()).andReturn(principalEntity).anyTimes();
     expect(principalEntity.getId()).andReturn(2L).anyTimes();

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

@@ -25,6 +25,8 @@ import static org.junit.Assert.assertTrue;
 import java.util.List;
 import java.util.Properties;
 
+import javax.persistence.EntityManager;
+
 import junit.framework.Assert;
 
 import org.apache.ambari.server.AmbariException;
@@ -39,14 +41,14 @@ import org.apache.ambari.server.orm.dao.ResourceDAO;
 import org.apache.ambari.server.orm.dao.ResourceTypeDAO;
 import org.apache.ambari.server.orm.dao.UserDAO;
 import org.apache.ambari.server.orm.entities.PermissionEntity;
-import org.apache.ambari.server.orm.entities.PrincipalEntity;
-import org.apache.ambari.server.orm.entities.PrincipalTypeEntity;
 import org.apache.ambari.server.orm.entities.ResourceEntity;
 import org.apache.ambari.server.orm.entities.ResourceTypeEntity;
 import org.apache.ambari.server.orm.entities.UserEntity;
 import org.junit.After;
 import org.junit.Before;
+import org.junit.Ignore;
 import org.junit.Test;
+import org.mockito.Mockito;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.context.SecurityContextHolder;
@@ -55,6 +57,7 @@ import org.springframework.security.crypto.password.PasswordEncoder;
 import com.google.inject.Guice;
 import com.google.inject.Inject;
 import com.google.inject.Injector;
+import com.google.inject.Provider;
 import com.google.inject.persist.PersistService;
 
 public class TestUsers {
@@ -80,6 +83,8 @@ public class TestUsers {
   protected PrincipalDAO principalDAO;
   @Inject
   protected PasswordEncoder passwordEncoder;
+  @Inject
+  Provider<EntityManager> entityManagerProvider;
   private Properties properties;
 
   @Before
@@ -91,6 +96,23 @@ public class TestUsers {
     injector.injectMembers(this);
     Authentication auth = new UsernamePasswordAuthenticationToken("admin", null);
     SecurityContextHolder.getContext().setAuthentication(auth);
+
+    // create admin permission
+    ResourceTypeEntity resourceTypeEntity = new ResourceTypeEntity();
+    resourceTypeEntity.setId(ResourceTypeEntity.AMBARI_RESOURCE_TYPE);
+    resourceTypeEntity.setName(ResourceTypeEntity.AMBARI_RESOURCE_TYPE_NAME);
+    resourceTypeDAO.create(resourceTypeEntity);
+
+    ResourceEntity resourceEntity = new ResourceEntity();
+    resourceEntity.setId(ResourceEntity.AMBARI_RESOURCE_ID);
+    resourceEntity.setResourceType(resourceTypeEntity);
+    resourceDAO.create(resourceEntity);
+
+    PermissionEntity adminPermissionEntity = new PermissionEntity();
+    adminPermissionEntity.setId(PermissionEntity.AMBARI_ADMIN_PERMISSION);
+    adminPermissionEntity.setPermissionName(PermissionEntity.AMBARI_ADMIN_PERMISSION_NAME);
+    adminPermissionEntity.setResourceType(resourceTypeEntity);
+    permissionDAO.create(adminPermissionEntity);
   }
 
   @After
@@ -116,12 +138,67 @@ public class TestUsers {
 
     assertEquals(2, userDAO.findAll().size());
 
-    UserEntity userEntity = userDAO.findLocalUserByName("user");
+    UserEntity userEntity = userDAO.findUserByName("user");
     assertNotNull("user", userEntity.getUserPassword());
 
     users.modifyPassword("user", "user", "resu");
 
-    assertNotSame(userEntity.getUserPassword(), userDAO.findLocalUserByName("user").getUserPassword());
+    assertNotSame(userEntity.getUserPassword(), userDAO.findUserByName("user").getUserPassword());
+  }
+
+  @Test
+  public void testGetAnyUser() throws Exception {
+    users.createUser("user", "user", true, false, false);
+    users.createUser("user_ldap", "user_ldap", true, false, true);
+
+    assertEquals("user", users.getAnyUser("user").getUserName());
+    assertEquals("user_ldap", users.getAnyUser("user_ldap").getUserName());
+    Assert.assertNull(users.getAnyUser("non_existing"));
+  }
+
+  @Test
+  public void testSetUserActive() throws Exception {
+    users.createUser("user", "user");
+
+    users.setUserActive("user", false);
+    Assert.assertEquals(false, users.getAnyUser("user").isActive());
+    users.setUserActive("user", true);
+    Assert.assertEquals(true, users.getAnyUser("user").isActive());
+
+    try {
+      users.setUserActive("fake user", true);
+      Assert.fail("It shouldn't be possible to call setUserActive() on non-existing user");
+    } catch (Exception ex) {
+    }
+  }
+
+  @Test
+  public void testSetUserLdap() throws Exception {
+    users.createUser("user", "user");
+    users.createUser("user_ldap", "user_ldap", true, false, true);
+
+    users.setUserLdap("user");
+    Assert.assertEquals(true, users.getAnyUser("user").isLdapUser());
+
+    try {
+      users.setUserLdap("fake user");
+      Assert.fail("It shouldn't be possible to call setUserLdap() on non-existing user");
+    } catch (AmbariException ex) {
+    }
+  }
+
+  @Test
+  public void testSetGroupLdap() throws Exception {
+    users.createGroup("group");
+
+    users.setGroupLdap("group");
+    Assert.assertTrue(users.getGroup("group").isLdapGroup());
+
+    try {
+      users.setGroupLdap("fake group");
+      Assert.fail("It shouldn't be possible to call setGroupLdap() on non-existing group");
+    } catch (AmbariException ex) {
+    }
   }
 
   @Test
@@ -174,6 +251,24 @@ public class TestUsers {
     assertEquals(1, groupDAO.findGroupByName(groupName).getMemberEntities().size());
   }
 
+  @Test
+  public void testGetAllMembers() throws Exception {
+    final String groupName = "engineering";
+    users.createGroup(groupName);
+    users.createUser("user1", "user1");
+    users.createUser("user2", "user2");
+    users.createUser("user3", "user3");
+    users.addMemberToGroup(groupName, "user1");
+    users.addMemberToGroup(groupName, "user2");
+    assertEquals(2, users.getAllMembers(groupName).size());
+
+    try {
+      users.getAllMembers("non existing");
+      Assert.fail("It shouldn't be possible to call getAllMembers() on non-existing group");
+    } catch (Exception ex) {
+    }
+  }
+
   @Test
   public void testRemoveMemberFromGroup() throws Exception {
     final String groupName = "engineering";
@@ -205,113 +300,169 @@ public class TestUsers {
     assertEquals(users.getGroupMembers("unexisting"), null);
   }
 
- @Test
- public void testModifyPassword_UserByHimselfPasswordOk() throws Exception {
-   Authentication auth = new UsernamePasswordAuthenticationToken("user", null);
-   SecurityContextHolder.getContext().setAuthentication(auth);
+  @Test
+  public void testModifyPassword_UserByHimselfPasswordOk() throws Exception {
+    Authentication auth = new UsernamePasswordAuthenticationToken("user", null);
+    SecurityContextHolder.getContext().setAuthentication(auth);
 
-   users.createUser("user", "user");
+    users.createUser("user", "user");
 
-   UserEntity userEntity = userDAO.findLocalUserByName("user");
+    UserEntity userEntity = userDAO.findUserByName("user");
 
-   assertNotSame("user", userEntity.getUserPassword());
-   assertTrue(passwordEncoder.matches("user", userEntity.getUserPassword()));
+    assertNotSame("user", userEntity.getUserPassword());
+    assertTrue(passwordEncoder.matches("user", userEntity.getUserPassword()));
 
-   users.modifyPassword("user", "user", "user_new_password");
+    users.modifyPassword("user", "user", "user_new_password");
 
-   assertTrue(passwordEncoder.matches("user_new_password", userDAO.findLocalUserByName("user").getUserPassword()));
- }
+    assertTrue(passwordEncoder.matches("user_new_password", userDAO.findUserByName("user").getUserPassword()));
+  }
 
- @Test
- public void testModifyPassword_UserByHimselfPasswordNotOk() throws Exception {
-   Authentication auth = new UsernamePasswordAuthenticationToken("user", null);
-   SecurityContextHolder.getContext().setAuthentication(auth);
+  @Test
+  public void testModifyPassword_UserByHimselfPasswordNotOk() throws Exception {
+    Authentication auth = new UsernamePasswordAuthenticationToken("user", null);
+    SecurityContextHolder.getContext().setAuthentication(auth);
 
-   users.createUser("user", "user");
+    users.createUser("user", "user");
 
-   UserEntity userEntity = userDAO.findLocalUserByName("user");
+    UserEntity userEntity = userDAO.findUserByName("user");
 
-   assertNotSame("user", userEntity.getUserPassword());
-   assertTrue(passwordEncoder.matches("user", userEntity.getUserPassword()));
+    assertNotSame("user", userEntity.getUserPassword());
+    assertTrue(passwordEncoder.matches("user", userEntity.getUserPassword()));
 
-   try {
-     users.modifyPassword("user", "admin", "user_new_password");
-     Assert.fail("Exception should be thrown here as password is incorrect");
-   } catch (AmbariException ex) {
-   }
- }
+    try {
+      users.modifyPassword("user", "admin", "user_new_password");
+      Assert.fail("Exception should be thrown here as password is incorrect");
+    } catch (AmbariException ex) {
+    }
+  }
 
- @Test
- public void testModifyPassword_UserByNonAdmin() throws Exception {
-   Authentication auth = new UsernamePasswordAuthenticationToken("user2", null);
-   SecurityContextHolder.getContext().setAuthentication(auth);
+  @Test
+  public void testModifyPassword_UserByNonAdmin() throws Exception {
+    Authentication auth = new UsernamePasswordAuthenticationToken("user2", null);
+    SecurityContextHolder.getContext().setAuthentication(auth);
 
-   users.createUser("user", "user");
-   users.createUser("user2", "user2");
+    users.createUser("user", "user");
+    users.createUser("user2", "user2");
 
-   UserEntity userEntity = userDAO.findLocalUserByName("user");
+    UserEntity userEntity = userDAO.findUserByName("user");
 
-   assertNotSame("user", userEntity.getUserPassword());
-   assertTrue(passwordEncoder.matches("user", userEntity.getUserPassword()));
+    assertNotSame("user", userEntity.getUserPassword());
+    assertTrue(passwordEncoder.matches("user", userEntity.getUserPassword()));
 
-   try {
-     users.modifyPassword("user", "user2", "user_new_password");
-     Assert.fail("Exception should be thrown here as user2 can't change password of user");
-   } catch (AmbariException ex) {
-   }
- }
+    try {
+      users.modifyPassword("user", "user2", "user_new_password");
+      Assert.fail("Exception should be thrown here as user2 can't change password of user");
+    } catch (AmbariException ex) {
+    }
+  }
 
- @Test
- public void testModifyPassword_UserByAdmin() throws Exception {
-   ResourceTypeEntity resourceTypeEntity = new ResourceTypeEntity();
-   resourceTypeEntity.setId(ResourceTypeEntity.AMBARI_RESOURCE_TYPE);
-   resourceTypeEntity.setName(ResourceTypeEntity.AMBARI_RESOURCE_TYPE_NAME);
-   resourceTypeDAO.create(resourceTypeEntity);
+  @Test
+  public void testModifyPassword_UserByAdmin() throws Exception {
+    users.createUser("admin", "admin", true, true, false);
+    users.createUser("user", "user");
 
-   ResourceEntity resourceEntity = new ResourceEntity();
-   resourceEntity.setId(ResourceEntity.AMBARI_RESOURCE_ID);
-   resourceEntity.setResourceType(resourceTypeEntity);
-   resourceDAO.create(resourceEntity);
+    UserEntity userEntity = userDAO.findUserByName("user");
 
-   PermissionEntity adminPermissionEntity = new PermissionEntity();
-   adminPermissionEntity.setId(PermissionEntity.AMBARI_ADMIN_PERMISSION);
-   adminPermissionEntity.setPermissionName(PermissionEntity.AMBARI_ADMIN_PERMISSION_NAME);
-   adminPermissionEntity.setResourceType(resourceTypeEntity);
-   permissionDAO.create(adminPermissionEntity);
+    assertNotSame("user", userEntity.getUserPassword());
+    assertTrue(passwordEncoder.matches("user", userEntity.getUserPassword()));
 
-   users.createUser("admin", "admin", true, true, false);
-   users.createUser("user", "user");
+    users.modifyPassword("user", "admin", "user_new_password");
+    assertTrue(passwordEncoder.matches("user_new_password", userDAO.findUserByName("user").getUserPassword()));
+  }
 
-   UserEntity userEntity = userDAO.findLocalUserByName("user");
+  @Test
+  public void testCreateUserTwoParams() throws Exception {
+    users.createUser("user", "user");
 
-   assertNotSame("user", userEntity.getUserPassword());
-   assertTrue(passwordEncoder.matches("user", userEntity.getUserPassword()));
+    final User createdUser = users.getAnyUser("user");
+    Assert.assertEquals("user", createdUser.getUserName());
+    Assert.assertEquals(true, createdUser.isActive());
+    Assert.assertEquals(false, createdUser.isLdapUser());
+    Assert.assertEquals(false, createdUser.isAdmin());
+  }
 
-   users.modifyPassword("user", "admin", "user_new_password");
-   assertTrue(passwordEncoder.matches("user_new_password", userDAO.findLocalUserByName("user").getUserPassword()));
- }
+  @Test
+  @Ignore // TODO @Transactional annotation breaks this test
+  public void testCreateUserDefaultParams() throws Exception {
+    final Users spy = Mockito.spy(users);
+    spy.createUser("user", "user");
+    Mockito.verify(spy).createUser("user", "user", true, false, false);
+  }
 
-  private void createLdapUser() {
+  @Test
+  public void testCreateUserFiveParams() throws Exception {
+    users.createUser("user", "user", false, false, false);
+
+    final User createdUser = users.getAnyUser("user");
+    Assert.assertEquals("user", createdUser.getUserName());
+    Assert.assertEquals(false, createdUser.isActive());
+    Assert.assertEquals(false, createdUser.isLdapUser());
+    Assert.assertEquals(false, createdUser.isAdmin());
+
+    users.createUser("user2", "user2", true, true, true);
+    final User createdUser2 = users.getAnyUser("user2");
+    Assert.assertEquals("user2", createdUser2.getUserName());
+    Assert.assertEquals(true, createdUser2.isActive());
+    Assert.assertEquals(true, createdUser2.isLdapUser());
+    Assert.assertEquals(true, createdUser2.isAdmin());
+  }
 
-    PrincipalTypeEntity principalTypeEntity = new PrincipalTypeEntity();
-    principalTypeEntity.setName(PrincipalTypeEntity.USER_PRINCIPAL_TYPE_NAME);
-    principalTypeDAO.create(principalTypeEntity);
+  @Test(expected = AmbariException.class)
+  public void testCreateUserDuplicate() throws Exception {
+    users.createUser("user", "user");
+    users.createUser("user", "user");
+  }
+
+  @Test
+  public void testRemoveUser() throws Exception {
+    users.createUser("user1", "user1");
+    users.createUser("user2", "user2");
+    users.createUser("user3", "user3");
+    Assert.assertEquals(3, users.getAllUsers().size());
+
+    users.removeUser(users.getAnyUser("user1"));
 
-    PrincipalEntity principalEntity = new PrincipalEntity();
-    principalEntity.setPrincipalType(principalTypeEntity);
-    principalDAO.create(principalEntity);
+    Assert.assertNull(users.getAnyUser("user1"));
+    Assert.assertEquals(2, users.getAllUsers().size());
+  }
 
-    UserEntity ldapUser = new UserEntity();
+  @Test
+  public void testGrantAdminPrivilege() throws Exception {
+    users.createUser("user", "user");
 
-    ldapUser.setUserName("ldapUser");
-    ldapUser.setLdapUser(true);
-    ldapUser.setPrincipal(principalEntity);
+    final User user = users.getAnyUser("user");
+    users.grantAdminPrivilege(user.getUserId());
 
-    userDAO.create(ldapUser);
+    Assert.assertTrue(users.getAnyUser("user").isAdmin());
+  }
 
-    UserEntity userEntity = userDAO.findLdapUserByName("ldapUser");
+  @Test
+  public void testRevokeAdminPrivilege() throws Exception {
+    users.createUser("admin", "admin", true, true, false);
+
+    final User admin = users.getAnyUser("admin");
+    users.revokeAdminPrivilege(admin.getUserId());
+
+    Assert.assertFalse(users.getAnyUser("admin").isAdmin());
+  }
+
+  @Test
+  public void testIsUserCanBeRemoved() throws Exception {
+    users.createUser("admin", "admin", true, true, false);
+    users.createUser("admin2", "admin2", true, true, false);
+
+    Assert.assertTrue(users.isUserCanBeRemoved(userDAO.findUserByName("admin")));
+    Assert.assertTrue(users.isUserCanBeRemoved(userDAO.findUserByName("admin2")));
+
+    users.removeUser(users.getAnyUser("admin"));
+
+    Assert.assertFalse(users.isUserCanBeRemoved(userDAO.findUserByName("admin2")));
+    users.createUser("user", "user");
+    Assert.assertFalse(users.isUserCanBeRemoved(userDAO.findUserByName("admin2")));
 
-    userDAO.merge(ldapUser);
+    users.createUser("admin3", "admin3", true, true, false);
+    Assert.assertTrue(users.isUserCanBeRemoved(userDAO.findUserByName("admin2")));
+    Assert.assertTrue(users.isUserCanBeRemoved(userDAO.findUserByName("admin3")));
   }
 
 }