Repository: ambari Updated Branches: refs/heads/branch-1.7.0 605852aa5 -> 9cd29804d
AMBARI-7717. Upgrade: Admin gets unnecessary READ permissions (alejandro) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/9cd29804 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/9cd29804 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/9cd29804 Branch: refs/heads/branch-1.7.0 Commit: 9cd29804d94ce60d56228c819bcbfc195d8ed119 Parents: 605852a Author: Alejandro Fernandez <[email protected]> Authored: Thu Oct 9 13:24:36 2014 -0700 Committer: Alejandro Fernandez <[email protected]> Committed: Fri Oct 10 10:42:10 2014 -0700 ---------------------------------------------------------------------- .../server/upgrade/UpgradeCatalog170.java | 47 ++++++++++---------- .../server/upgrade/UpgradeCatalog170Test.java | 41 ++++++++++++++--- 2 files changed, 59 insertions(+), 29 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/9cd29804/ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog170.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog170.java b/ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog170.java index 1de0061..c874a79 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog170.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog170.java @@ -1264,34 +1264,33 @@ public class UpgradeCatalog170 extends AbstractUpgradeCatalog { } for (UserEntity user: userDAO.findAll()) { - for (String role: roles.get(user)) { - if (role.equals("admin")) { + List<String> userRoles = roles.get(user); + if (userRoles.contains("admin")) { + final PrivilegeEntity privilege = new PrivilegeEntity(); + privilege.setPermission(adminPermission); + privilege.setPrincipal(user.getPrincipal()); + privilege.setResource(ambariResource); + user.getPrincipal().getPrivileges().add(privilege); + privilegeDAO.create(privilege); + for (ClusterEntity cluster: clusterDAO.findAll()) { + final PrivilegeEntity clusterPrivilege = new PrivilegeEntity(); + clusterPrivilege.setPermission(clusterOperatePermission); + clusterPrivilege.setPrincipal(user.getPrincipal()); + clusterPrivilege.setResource(cluster.getResource()); + privilegeDAO.create(clusterPrivilege); + user.getPrincipal().getPrivileges().add(clusterPrivilege); + } + userDAO.merge(user); + } else if (userRoles.contains("user")) { + for (ClusterEntity cluster: clusterDAO.findAll()) { final PrivilegeEntity privilege = new PrivilegeEntity(); - privilege.setPermission(adminPermission); + privilege.setPermission(clusterReadPermission); privilege.setPrincipal(user.getPrincipal()); - privilege.setResource(ambariResource); - user.getPrincipal().getPrivileges().add(privilege); + privilege.setResource(cluster.getResource()); privilegeDAO.create(privilege); - for (ClusterEntity cluster: clusterDAO.findAll()) { - final PrivilegeEntity clusterPrivilege = new PrivilegeEntity(); - clusterPrivilege.setPermission(clusterOperatePermission); - clusterPrivilege.setPrincipal(user.getPrincipal()); - clusterPrivilege.setResource(cluster.getResource()); - privilegeDAO.create(clusterPrivilege); - user.getPrincipal().getPrivileges().add(clusterPrivilege); - } - userDAO.merge(user); - } else if (role.equals("user")) { - for (ClusterEntity cluster: clusterDAO.findAll()) { - final PrivilegeEntity privilege = new PrivilegeEntity(); - privilege.setPermission(clusterReadPermission); - privilege.setPrincipal(user.getPrincipal()); - privilege.setResource(cluster.getResource()); - privilegeDAO.create(privilege); - user.getPrincipal().getPrivileges().add(privilege); - } - userDAO.merge(user); + user.getPrincipal().getPrivileges().add(privilege); } + userDAO.merge(user); } } http://git-wip-us.apache.org/repos/asf/ambari/blob/9cd29804/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog170Test.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog170Test.java b/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog170Test.java index ef4ea79..57ddd40 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog170Test.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog170Test.java @@ -69,6 +69,7 @@ import org.apache.ambari.server.orm.entities.HostEntity; import org.apache.ambari.server.orm.entities.HostRoleCommandEntity; import org.apache.ambari.server.orm.entities.KeyValueEntity; import org.apache.ambari.server.orm.entities.PermissionEntity; +import org.apache.ambari.server.orm.entities.PrincipalEntity; import org.apache.ambari.server.orm.entities.PrivilegeEntity; import org.apache.ambari.server.orm.entities.ResourceEntity; import org.apache.ambari.server.orm.entities.ResourceTypeEntity; @@ -109,6 +110,7 @@ import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -522,7 +524,6 @@ public class UpgradeCatalog170Test { expect(entityManager.getTransaction()).andReturn(trans).anyTimes(); expect(entityManager.getCriteriaBuilder()).andReturn(cb).anyTimes(); expect(entityManager.createQuery(cq)).andReturn(q).anyTimes(); - expect(userRolesResultSet.next()).andReturn(false).once(); expect(trans.isActive()).andReturn(true).anyTimes(); expect(upgradeCatalog.getEntityManagerProvider()).andReturn(entityManagerProvider).anyTimes(); expect(cb.createQuery(HostRoleCommandEntity.class)).andReturn(cq).anyTimes(); @@ -570,11 +571,38 @@ public class UpgradeCatalog170Test { expect(injector.getInstance(KeyValueDAO.class)).andReturn(keyValueDAO).anyTimes(); expect(injector.getInstance(ViewRegistry.class)).andReturn(viewRegistry).anyTimes(); + expect(userRolesResultSet.next()).andReturn(true).times(3); + expect(userRolesResultSet.next()).andReturn(false).times(1); + expect(userRolesResultSet.getString(1)).andReturn("admin").times(1); + expect(userRolesResultSet.getString(1)).andReturn("user").times(2); + expect(userRolesResultSet.getInt(2)).andReturn(1).times(2); + expect(userRolesResultSet.getInt(2)).andReturn(2).times(1); + + UserEntity userEntity1 = createNiceMock(UserEntity.class); + UserEntity userEntity2 = createNiceMock(UserEntity.class); + PrincipalEntity userPrincipal1 = createNiceMock(PrincipalEntity.class); + PrincipalEntity userPrincipal2 = createNiceMock(PrincipalEntity.class); + Set<PrivilegeEntity> userPrivileges1 = createNiceMock(Set.class); + Set<PrivilegeEntity> userPrivileges2 = createNiceMock(Set.class); + expect(userEntity1.getPrincipal()).andReturn(userPrincipal1).anyTimes(); + expect(userEntity2.getPrincipal()).andReturn(userPrincipal2).anyTimes(); + expect(userPrincipal1.getPrivileges()).andReturn(userPrivileges1).anyTimes(); + expect(userPrincipal2.getPrivileges()).andReturn(userPrivileges2).anyTimes(); + expect(userPrivileges1.add(anyObject(PrivilegeEntity.class))).andReturn(true).once(); + expect(userDAO.findByPK(1)).andReturn(userEntity1).times(2); + expect(userDAO.findByPK(2)).andReturn(userEntity2).once(); + expect(userDAO.merge(userEntity1)).andReturn(userEntity1).once(); + expect(userDAO.merge(userEntity2)).andReturn(userEntity2).once(); + + expect(configGroupConfigMappingDAO.findAll()).andReturn(configGroupConfigMappingEntities).once(); + expect(userDAO.findAll()).andReturn(Collections.<UserEntity> emptyList()).times(1); + expect(userDAO.findAll()).andReturn(Arrays.asList(userEntity1, userEntity2)).times(1); + expect(clusterDAO.findAll()).andReturn(Collections.<ClusterEntity> emptyList()).times(1); + String yarnConfig = String.format("{'%s':'%s', '%s':'%s'}", YARN_TIMELINE_SERVICE_WEBAPP_ADDRESS_PROPERTY, "timeline:8081", YARN_RESOURCEMANAGER_WEBAPP_ADDRESS_PROPERTY, "resource_man:8081"); expect(configGroupConfigMappingDAO.findAll()).andReturn(configGroupConfigMappingEntities).once(); - expect(userDAO.findAll()).andReturn(Collections.<UserEntity>emptyList()).times(2); expect(clusterDAO.findAll()).andReturn(Collections.singletonList(clusterEntity)).anyTimes(); expect(configEntity.getData()).andReturn(yarnConfig); expect(clusterDAO.findConfig(1L, YARN_SITE, "version1")).andReturn(configEntity).anyTimes(); @@ -629,7 +657,9 @@ public class UpgradeCatalog170Test { replay(dbAccessor, configuration, injector, cluster, clusters, amc, config, configHelper, pigConfig); replay(userDAO, clusterDAO, viewDAO, viewInstanceDAO, permissionDAO, configGroupConfigMappingDAO); replay(resourceTypeDAO, resourceDAO, keyValueDAO, privilegeDAO, clusterConfigEntity); - replay(jobsView, showJobsKeyValue, user, viewRegistry, viewUsePermission, adminPermission); + replay(jobsView, showJobsKeyValue, user); + replay(userEntity1, userEntity2, userPrincipal1, userPrincipal2, userPrivileges1, userPrivileges2); + replay(viewRegistry, viewUsePermission, adminPermission); replay(clusterEntity, configEntity, configMappingEntity, clusterStateEntity); Class<?> c = AbstractUpgradeCatalog.class; @@ -646,8 +676,9 @@ public class UpgradeCatalog170Test { upgradeCatalog.executeDMLUpdates(); verify(upgradeCatalog, dbAccessor, configuration, injector, cluster, clusters, amc, config, configHelper, - jobsView, showJobsKeyValue, privilegeDAO, viewDAO, viewInstanceDAO, resourceDAO, keyValueDAO, - viewRegistry, userRolesResultSet, clusterEntity, configEntity, configMappingEntity, clusterStateEntity); + jobsView, showJobsKeyValue, privilegeDAO, viewDAO, viewInstanceDAO, resourceDAO, keyValueDAO, userRolesResultSet, + userEntity1, userEntity2, userPrincipal1, userPrincipal2, userPrivileges1, userPrivileges2, + viewRegistry, clusterEntity, configEntity, configMappingEntity, clusterStateEntity); }
