Repository: ambari Updated Branches: refs/heads/trunk 0c627bc98 -> 85a2728a2
AMBARI-20482. Accessing a user after sync with AD is failing with 500. (Balazs Bence Sari via stoader) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/85a2728a Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/85a2728a Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/85a2728a Branch: refs/heads/trunk Commit: 85a2728a2190414975487b05d556d1fec89243c0 Parents: 0c627bc Author: Balazs Bence Sari <bs...@hortonworks.com> Authored: Fri Mar 17 17:07:56 2017 +0100 Committer: Toader, Sebastian <stoa...@hortonworks.com> Committed: Fri Mar 17 17:07:56 2017 +0100 ---------------------------------------------------------------------- .../ActiveWidgetLayoutResourceProvider.java | 2 +- .../apache/ambari/server/orm/dao/UserDAO.java | 38 +++++++ .../server/security/authorization/Users.java | 23 ++-- .../ActiveWidgetLayoutResourceProviderTest.java | 2 +- .../ambari/server/orm/dao/UserDAOTest.java | 107 +++++++++++++++++-- .../security/authorization/TestUsers.java | 8 +- .../security/authorization/UsersTest.java | 66 +++++++++++- 7 files changed, 207 insertions(+), 39 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/85a2728a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ActiveWidgetLayoutResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ActiveWidgetLayoutResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ActiveWidgetLayoutResourceProvider.java index d9a7997..fe45e03 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ActiveWidgetLayoutResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ActiveWidgetLayoutResourceProvider.java @@ -159,7 +159,7 @@ public class ActiveWidgetLayoutResourceProvider extends AbstractControllerResour } java.lang.reflect.Type type = new TypeToken<Set<Map<String, String>>>(){}.getType(); - Set<Map<String, String>> activeWidgetLayouts = gson.fromJson(userDAO.findUserByName(userName).getActiveWidgetLayouts(), type); + Set<Map<String, String>> activeWidgetLayouts = gson.fromJson(userDAO.findSingleUserByName(userName).getActiveWidgetLayouts(), type); if (activeWidgetLayouts != null) { for (Map<String, String> widgetLayoutId : activeWidgetLayouts) { layoutEntities.add(widgetLayoutDAO.findById(Long.parseLong(widgetLayoutId.get(ID)))); http://git-wip-us.apache.org/repos/asf/ambari/blob/85a2728a/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UserDAO.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UserDAO.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UserDAO.java index 5659cc5..60f6fd5 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UserDAO.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UserDAO.java @@ -23,6 +23,7 @@ import java.util.HashSet; import java.util.List; import java.util.Set; +import javax.annotation.Nullable; import javax.persistence.EntityManager; import javax.persistence.NoResultException; import javax.persistence.TypedQuery; @@ -32,6 +33,7 @@ import org.apache.ambari.server.orm.entities.PrincipalEntity; import org.apache.ambari.server.orm.entities.UserEntity; import org.apache.ambari.server.security.authorization.UserType; +import com.google.common.collect.ImmutableMap; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; @@ -72,6 +74,42 @@ public class UserDAO { } } + /** + * <p>Finds user by name. If duplicate users exists (with different type), the returned one will be chosen by this user + * type precedence: LOCAL -> LDAP -> JWT -> PAM</p> + * <p>In Ambari 3.0, user management will be rethought hence the deprecation</p> + * @param userName the user name + * @return The corresponding user or {@code null} if none is found. If multiple users exist with different types, user + * type precedence (see above) will decide. + */ + @RequiresSession + @Deprecated + @Nullable + public UserEntity findSingleUserByName(String userName) { + TypedQuery<UserEntity> query = entityManagerProvider.get().createNamedQuery("userByName", UserEntity.class); + query.setParameter("username", userName.toLowerCase()); + List<UserEntity> resultList = query.getResultList(); + switch (resultList.size()) { + case 0: + return null; + case 1: + return resultList.get(0); + default: + ImmutableMap.Builder<UserType, UserEntity> mapBuilder = ImmutableMap.<UserType, UserEntity>builder(); + for (UserEntity user: resultList) { + mapBuilder.put(user.getUserType(), user); + } + ImmutableMap<UserType, UserEntity> usersByType = mapBuilder.build(); + UserEntity user = + usersByType.containsKey(UserType.LOCAL) ? usersByType.get(UserType.LOCAL) : + usersByType.containsKey(UserType.LOCAL.LDAP) ? usersByType.get(UserType.LDAP) : + usersByType.containsKey(UserType.JWT) ? usersByType.get(UserType.JWT) : + usersByType.get(UserType.PAM); + return user; + } + } + + @RequiresSession public UserEntity findUserByNameAndType(String userName, UserType userType) { TypedQuery<UserEntity> query = entityManagerProvider.get().createQuery("SELECT user FROM UserEntity user WHERE " + http://git-wip-us.apache.org/repos/asf/ambari/blob/85a2728a/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java b/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java index e69bbc9..fbd40a0 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java @@ -120,22 +120,11 @@ public class Users { /** * This method works incorrectly, userName is not unique if users have different types * - * @return One user. Priority is LOCAL -> LDAP -> JWT + * @return One user. Priority is LOCAL -> LDAP -> JWT -> PAM */ @Deprecated public User getAnyUser(String userName) { - UserEntity userEntity = userDAO.findUserByNameAndType(userName, UserType.LOCAL); - if (userEntity == null) { - userEntity = userDAO.findUserByNameAndType(userName, UserType.LDAP); - } - if (userEntity == null) { - userEntity = userDAO.findUserByNameAndType(userName, UserType.JWT); - } - - if (userEntity == null) { - userEntity = userDAO.findUserByNameAndType(userName, UserType.PAM); - } - + UserEntity userEntity = userDAO.findSingleUserByName(userName); return (null == userEntity) ? null : new User(userEntity); } @@ -153,9 +142,12 @@ public class Users { * Retrieves User then userName is unique in users DB. Will return null if there no user with provided userName or * there are some users with provided userName but with different types. * + * <p>User names in the future will likely be unique hence the deprecation.</p> + * * @param userName * @return User if userName is unique in DB, null otherwise */ + @Deprecated public User getUserIfUnique(String userName) { List<UserEntity> userEntities = new ArrayList<>(); UserEntity userEntity = userDAO.findUserByNameAndType(userName, UserType.LOCAL); @@ -311,9 +303,10 @@ public class Users { throw new AmbariException("UserType not specified."); } - User existingUser = getUser(userName, userType); + User existingUser = getAnyUser(userName); if (existingUser != null) { - throw new AmbariException("User " + existingUser.getUserName() + " already exists"); + throw new AmbariException("User " + existingUser.getUserName() + " already exists with type " + + existingUser.getUserType()); } PrincipalTypeEntity principalTypeEntity = principalTypeDAO.findById(PrincipalTypeEntity.USER_PRINCIPAL_TYPE); http://git-wip-us.apache.org/repos/asf/ambari/blob/85a2728a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ActiveWidgetLayoutResourceProviderTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ActiveWidgetLayoutResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ActiveWidgetLayoutResourceProviderTest.java index e370a8a..4dc06b9 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ActiveWidgetLayoutResourceProviderTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ActiveWidgetLayoutResourceProviderTest.java @@ -172,7 +172,7 @@ public class ActiveWidgetLayoutResourceProviderTest extends EasyMockSupport { UserEntity userEntity = createMockUserEntity(requestedUsername); UserDAO userDAO = injector.getInstance(UserDAO.class); - expect(userDAO.findUserByName(requestedUsername)).andReturn(userEntity).atLeastOnce(); + expect(userDAO.findSingleUserByName(requestedUsername)).andReturn(userEntity).atLeastOnce(); WidgetLayoutDAO widgetLayoutDAO = injector.getInstance(WidgetLayoutDAO.class); expect(widgetLayoutDAO.findById(1L)).andReturn(createMockWidgetLayout(1L, requestedUsername)).atLeastOnce(); http://git-wip-us.apache.org/repos/asf/ambari/blob/85a2728a/ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UserDAOTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UserDAOTest.java b/ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UserDAOTest.java index a1ca11d..bb0b0cf 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UserDAOTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UserDAOTest.java @@ -18,16 +18,29 @@ package org.apache.ambari.server.orm.dao; +import static org.easymock.EasyMock.anyObject; +import static org.easymock.EasyMock.anyString; +import static org.easymock.EasyMock.createNiceMock; import static org.easymock.EasyMock.createStrictMock; import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.replay; -import static org.easymock.EasyMock.reset; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + +import java.util.Arrays; import javax.persistence.EntityManager; +import javax.persistence.TypedQuery; -import org.junit.Before; +import org.apache.ambari.server.orm.DBAccessor; +import org.apache.ambari.server.orm.entities.UserEntity; +import org.apache.ambari.server.security.authorization.UserType; +import org.junit.Test; +import com.google.inject.AbstractModule; +import com.google.inject.Guice; import com.google.inject.Inject; +import com.google.inject.Injector; import com.google.inject.Provider; /** @@ -35,17 +48,89 @@ import com.google.inject.Provider; */ public class UserDAOTest { - @Inject - DaoUtils daoUtils; + private static String SERVICEOP_USER_NAME = "serviceopuser"; + private UserDAO userDAO; + + public void init(UserEntity... usersInDB) { + final EntityManager entityManager = createStrictMock(EntityManager.class); + final DaoUtils daoUtils = createNiceMock(DaoUtils.class); + final DBAccessor dbAccessor = createNiceMock(DBAccessor.class); + final Injector mockInjector = Guice.createInjector(new AbstractModule() { + @Override + protected void configure() { + bind(EntityManagerProvider.class); + bind(EntityManager.class).toInstance(entityManager); + bind(DBAccessor.class).toInstance(dbAccessor); + bind(DaoUtils.class).toInstance(daoUtils); + } + }); + userDAO = mockInjector.getInstance(UserDAO.class); + + TypedQuery<UserEntity> userQuery = createNiceMock(TypedQuery.class); + expect(userQuery.getResultList()).andReturn(Arrays.asList(usersInDB)); + expect(entityManager.createNamedQuery(anyString(), anyObject(Class.class))).andReturn(userQuery); + replay(entityManager, daoUtils, dbAccessor, userQuery); + } + + @Test + public void testFindSingleUserByName_NoUsers() { + init(); + assertNull(userDAO.findSingleUserByName(SERVICEOP_USER_NAME)); + } + + @Test + public void testFindSingleUserByName_SingleUser() { + init(user(UserType.PAM)); + assertEquals(UserType.PAM, userDAO.findSingleUserByName(SERVICEOP_USER_NAME).getUserType()); + } + + @Test + public void testFindSingleUserByName_LocalIsFirstPrecedence() { + init(user(UserType.LOCAL), + user(UserType.LDAP), + user(UserType.JWT), + user(UserType.PAM)); + assertEquals(UserType.LOCAL, userDAO.findSingleUserByName(SERVICEOP_USER_NAME).getUserType()); + } + + @Test + public void testFindSingleUserByName_LdapIsSecondPrecedence() { + init(user(UserType.LDAP), + user(UserType.JWT), + user(UserType.PAM)); + assertEquals(UserType.LDAP, userDAO.findSingleUserByName(SERVICEOP_USER_NAME).getUserType()); + } + + @Test + public void testFindSingleUserByName_JwtIsThirdPrecedence() { + init(user(UserType.JWT), + user(UserType.PAM)); + assertEquals(UserType.JWT, userDAO.findSingleUserByName(SERVICEOP_USER_NAME).getUserType()); + } + + private static final UserEntity user(UserType type) { + return user(SERVICEOP_USER_NAME, type); + } + + private static final UserEntity user(String name, UserType type) { + UserEntity userEntity = new UserEntity(); + userEntity.setUserName(name); + userEntity.setUserType(type); + return userEntity; + } + + static class EntityManagerProvider implements Provider<EntityManager> { + private final EntityManager entityManager; - Provider<EntityManager> entityManagerProvider = createStrictMock(Provider.class); - EntityManager entityManager = createStrictMock(EntityManager.class); + @Inject + public EntityManagerProvider(EntityManager entityManager) { + this.entityManager = entityManager; + } - @Before - public void init() { - reset(entityManagerProvider); - expect(entityManagerProvider.get()).andReturn(entityManager).atLeastOnce(); - replay(entityManagerProvider); + @Override + public EntityManager get() { + return entityManager; + } } } http://git-wip-us.apache.org/repos/asf/ambari/blob/85a2728a/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/TestUsers.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/TestUsers.java b/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/TestUsers.java index 2ca5396..e29791f 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/TestUsers.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/TestUsers.java @@ -241,17 +241,13 @@ public class TestUsers { // get user if unique Assert.assertNotNull(users.getUserIfUnique("user")); - users.createUser("user", "user", UserType.LDAP, true, false); - - Assert.assertNull(users.getUserIfUnique("user")); - //remove user - Assert.assertEquals(4, users.getAllUsers().size()); + Assert.assertEquals(3, users.getAllUsers().size()); users.removeUser(users.getAnyUser("user1")); Assert.assertNull(users.getAnyUser("user1")); - Assert.assertEquals(3, users.getAllUsers().size()); + Assert.assertEquals(2, users.getAllUsers().size()); } @Test http://git-wip-us.apache.org/repos/asf/ambari/blob/85a2728a/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/UsersTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/UsersTest.java b/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/UsersTest.java index 0364213..dcb3109 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/UsersTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/UsersTest.java @@ -18,8 +18,12 @@ package org.apache.ambari.server.security.authorization; +import static org.easymock.EasyMock.anyObject; +import static org.easymock.EasyMock.anyString; import static org.easymock.EasyMock.capture; +import static org.easymock.EasyMock.eq; import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.expectLastCall; import static org.easymock.EasyMock.newCapture; import java.util.ArrayList; @@ -27,12 +31,15 @@ import java.util.Collection; import java.util.Collections; import java.util.List; +import javax.annotation.Nullable; import javax.persistence.EntityManager; +import org.apache.ambari.server.AmbariException; import org.apache.ambari.server.hooks.HookContextFactory; import org.apache.ambari.server.hooks.HookService; import org.apache.ambari.server.orm.DBAccessor; import org.apache.ambari.server.orm.dao.MemberDAO; +import org.apache.ambari.server.orm.dao.PrincipalDAO; import org.apache.ambari.server.orm.dao.PrivilegeDAO; import org.apache.ambari.server.orm.dao.UserDAO; import org.apache.ambari.server.orm.entities.GroupEntity; @@ -43,6 +50,7 @@ import org.apache.ambari.server.orm.entities.PrivilegeEntity; import org.apache.ambari.server.orm.entities.UserEntity; import org.apache.ambari.server.state.stack.OsFamily; import org.easymock.Capture; +import org.easymock.EasyMock; import org.easymock.EasyMockSupport; import org.junit.Test; import org.springframework.security.crypto.password.PasswordEncoder; @@ -55,10 +63,12 @@ import junit.framework.Assert; public class UsersTest extends EasyMockSupport { + public static final String SERVICEOP_USER_NAME = "serviceopuser"; + private Injector injector; @Test public void testGetUserAuthorities() throws Exception { - Injector injector = getInjector(); + createInjector(); PrincipalEntity userPrincipalEntity = createMock(PrincipalEntity.class); @@ -134,19 +144,65 @@ public class UsersTest extends EasyMockSupport { Assert.assertTrue(authorities.contains(new AmbariGrantedAuthority(clusterUserViewUserPrivilegeEntity))); } - private Injector getInjector() { - return Guice.createInjector(new AbstractModule() { + /** + * User creation should complete without exception in case of unique user name + */ + @Test + public void testCreateUser_NoDuplicates() throws Exception { + initForCreateUser(null); + Users users = injector.getInstance(Users.class); + users.createUser(SERVICEOP_USER_NAME, "qwert"); + } + + /** + * User creation should throw {@link AmbariException} in case another user exists with the same name but + * different user type. + */ + @Test(expected = AmbariException.class) + public void testCreateUser_Duplicate() throws Exception { + UserEntity existing = new UserEntity(); + existing.setUserName(SERVICEOP_USER_NAME); + existing.setUserType(UserType.LDAP); + existing.setUserId(1); + existing.setMemberEntities(Collections.<MemberEntity>emptySet()); + PrincipalEntity principal = new PrincipalEntity(); + principal.setPrivileges(Collections.<PrivilegeEntity>emptySet()); + existing.setPrincipal(principal); + initForCreateUser(existing); + + Users users = injector.getInstance(Users.class); + users.createUser(SERVICEOP_USER_NAME, "qwert"); + } + + private void initForCreateUser(@Nullable UserEntity existingUser) { + UserDAO userDao = createStrictMock(UserDAO.class); + expect(userDao.findSingleUserByName(anyString())).andReturn(existingUser); + userDao.create(anyObject(UserEntity.class)); + expectLastCall(); + EntityManager entityManager = createNiceMock(EntityManager.class); + expect(entityManager.find(eq(PrincipalEntity.class), EasyMock.anyObject())).andReturn(null); + replayAll(); + createInjector(userDao, entityManager); + } + + private void createInjector() { + createInjector(createMock(UserDAO.class), createMock(EntityManager.class)); + } + + private void createInjector(final UserDAO mockUserDao, final EntityManager mockEntityManager) { + injector = Guice.createInjector(new AbstractModule() { @Override protected void configure() { - bind(EntityManager.class).toInstance(createMock(EntityManager.class)); + bind(EntityManager.class).toInstance(mockEntityManager); bind(DBAccessor.class).toInstance(createMock(DBAccessor.class)); bind(OsFamily.class).toInstance(createNiceMock(OsFamily.class)); - bind(UserDAO.class).toInstance(createMock(UserDAO.class)); + bind(UserDAO.class).toInstance(mockUserDao); bind(MemberDAO.class).toInstance(createMock(MemberDAO.class)); bind(PrivilegeDAO.class).toInstance(createMock(PrivilegeDAO.class)); bind(PasswordEncoder.class).toInstance(createMock(PasswordEncoder.class)); bind(HookService.class).toInstance(createMock(HookService.class)); bind(HookContextFactory.class).toInstance(createMock(HookContextFactory.class)); + bind(PrincipalDAO.class).toInstance(createMock(PrincipalDAO.class)); } }); }
