diff --git a/api/src/com/cloud/api/commands/ListVMsCmd.java b/api/src/com/cloud/api/commands/ListVMsCmd.java index f6351361b29..ca96b338ae0 100755 --- a/api/src/com/cloud/api/commands/ListVMsCmd.java +++ b/api/src/com/cloud/api/commands/ListVMsCmd.java @@ -47,7 +47,7 @@ public class ListVMsCmd extends BaseListCmd { @Parameter(name=ApiConstants.DOMAIN_ID, type=CommandType.LONG, description="the domain ID. If used with the account parameter, lists virtual machines for the specified account in this domain.") private Long domainId; - @Parameter(name=ApiConstants.IS_RECURSIVE, type=CommandType.BOOLEAN, description="defaults to false, but if true, lists all vms from the parent specified by the domain id till leaves.") + @Parameter(name=ApiConstants.IS_RECURSIVE, type=CommandType.BOOLEAN, description="Must be used with domainId parameter. Defaults to false, but if true, lists all vms from the parent specified by the domain id till leaves.") private Boolean recursive; @Parameter(name=ApiConstants.GROUP_ID, type=CommandType.LONG, description="the group ID") diff --git a/server/src/com/cloud/acl/DomainChecker.java b/server/src/com/cloud/acl/DomainChecker.java index 720e44a226d..ee98e80b3ce 100755 --- a/server/src/com/cloud/acl/DomainChecker.java +++ b/server/src/com/cloud/acl/DomainChecker.java @@ -53,7 +53,11 @@ public class DomainChecker extends AdapterBase implements SecurityChecker { throw new PermissionDeniedException(account + " is disabled."); } - if (!_domainDao.isChildDomain(account.getDomainId(), domain.getId())) { + if (account.getType() == Account.ACCOUNT_TYPE_NORMAL) { + if (account.getDomainId() != domain.getId()) { + throw new PermissionDeniedException(account + " does not have permission to operate within " + domain); + } + } else if (!_domainDao.isChildDomain(account.getDomainId(), domain.getId())) { throw new PermissionDeniedException(account + " does not have permission to operate within " + domain); } diff --git a/server/src/com/cloud/api/ApiDBUtils.java b/server/src/com/cloud/api/ApiDBUtils.java index 26446a73360..571226d8040 100755 --- a/server/src/com/cloud/api/ApiDBUtils.java +++ b/server/src/com/cloud/api/ApiDBUtils.java @@ -299,7 +299,7 @@ public class ApiDBUtils { ///////////////////////////////////////////////////////////// public static Account findAccountById(Long accountId) { - return _accountDao.findById(accountId); + return _accountDao.findByIdIncludingRemoved(accountId); } public static Account findAccountByIdIncludingRemoved(Long accountId) { diff --git a/server/src/com/cloud/api/ApiResponseHelper.java b/server/src/com/cloud/api/ApiResponseHelper.java index 7a0d931b18b..6507d3870b4 100755 --- a/server/src/com/cloud/api/ApiResponseHelper.java +++ b/server/src/com/cloud/api/ApiResponseHelper.java @@ -995,17 +995,10 @@ public class ApiResponseHelper implements ResponseGenerator { public UserVmResponse createUserVmResponse(UserVm userVm) { UserVmResponse userVmResponse = new UserVmResponse(); Account acct = ApiDBUtils.findAccountById(Long.valueOf(userVm.getAccountId())); - // FIXME - this check should be done in searchForUserVm method in - // ManagementServerImpl; - // otherwise the number of vms returned is not going to match pageSize - // request parameter - if ((acct != null) && (acct.getRemoved() == null)) { + if (acct != null) { userVmResponse.setAccountName(acct.getAccountName()); userVmResponse.setDomainId(acct.getDomainId()); userVmResponse.setDomainName(ApiDBUtils.findDomainById(acct.getDomainId()).getName()); - } else { - return null; // the account has been deleted, skip this VM in the - // response } userVmResponse.setId(userVm.getId()); diff --git a/server/src/com/cloud/vm/UserVmManagerImpl.java b/server/src/com/cloud/vm/UserVmManagerImpl.java index 2198d4dad09..266c29cda2f 100755 --- a/server/src/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/com/cloud/vm/UserVmManagerImpl.java @@ -2621,58 +2621,57 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager @Override public List searchForUserVMs(ListVMsCmd cmd) throws InvalidParameterValueException, PermissionDeniedException { - Account account = UserContext.current().getCaller(); + Account caller = UserContext.current().getCaller(); Long domainId = cmd.getDomainId(); String accountName = cmd.getAccountName(); - Long accountId = null; Boolean isRecursive = cmd.isRecursive(); String hypervisor = cmd.getHypervisor(); - List domainsToSearchForVms = new ArrayList(); - boolean isAdmin = false; + Long accountId = null; String path = null; - if ((account == null) || isAdmin(account.getType())) { - isAdmin = true; - if (domainId != null) { - if ((account != null) && !_domainDao.isChildDomain(account.getDomainId(), domainId)) { - throw new PermissionDeniedException("Invalid domain id (" + domainId + ") given, unable to list virtual machines."); - } - - if (accountName != null) { - account = _accountDao.findActiveAccount(accountName, domainId); - if (account == null) { - throw new InvalidParameterValueException("Unable to find account " + accountName + " in domain " + domainId); - } - accountId = account.getId(); - } - } - if (account.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) { - DomainVO domain = _domainDao.findById(account.getDomainId()); - if (domain != null) { - path = domain.getPath(); - } - } - } else { - accountId = account.getId(); - } - - if(isRecursive == null) { - isRecursive = false; - } - if(isRecursive && domainId != null) { - DomainVO parentDomain = _domainDao.findById(domainId); - if(parentDomain.getName().equals("ROOT")) { - domainsToSearchForVms.addAll(_domainDao.listAll()); - return recursivelySearchForVms(cmd, path, isAdmin, domainsToSearchForVms, accountId); - }else { - domainsToSearchForVms.add(parentDomain); - domainsToSearchForVms.addAll(_domainDao.findAllChildren(parentDomain.getPath(), parentDomain.getId())); - return recursivelySearchForVms(cmd, path, isAdmin, domainsToSearchForVms, accountId); - } - } else if(isRecursive && domainId == null){ + if (isRecursive != null && isRecursive && domainId == null){ throw new InvalidParameterValueException("Please enter a parent domain id for listing vms recursively"); } - + + if (domainId != null) { + //Verify if user is authorized to see instances belonging to the domain + DomainVO domain = _domainDao.findById(domainId); + if (domain == null) { + throw new InvalidParameterValueException("Domain id=" + domainId + " doesn't exist"); + } + _accountMgr.checkAccess(caller, domain); + } + + boolean isAdmin = false; + + if (_accountMgr.isAdmin(caller.getType())) { + isAdmin = true; + if (accountName != null && domainId != null) { + caller = _accountDao.findActiveAccount(accountName, domainId); + if (caller == null) { + throw new InvalidParameterValueException("Unable to find account " + accountName + " in domain " + domainId); + } + accountId = caller.getId(); + } + + if (caller.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) { + if (isRecursive == null) { + DomainVO domain = _domainDao.findById(caller.getDomainId()); + path = domain.getPath(); + } + } + } else { + accountId = caller.getId(); + } + + if (isRecursive != null && isRecursive && isAdmin) { + if (isRecursive) { + DomainVO domain = _domainDao.findById(domainId); + path = domain.getPath(); + domainId = null; + } + } + Criteria c = new Criteria("id", Boolean.TRUE, cmd.getStartIndex(), cmd.getPageSizeVal()); c.addCriteria(Criteria.KEYWORD, cmd.getKeyword()); c.addCriteria(Criteria.ID, cmd.getId()); @@ -2683,19 +2682,22 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager c.addCriteria(Criteria.FOR_VIRTUAL_NETWORK, cmd.getForVirtualNetwork()); c.addCriteria(Criteria.NETWORKID, cmd.getNetworkId()); + if (domainId != null) { + c.addCriteria(Criteria.DOMAINID, domainId); + } + if (path != null) { c.addCriteria(Criteria.PATH, path); } if (HypervisorType.getType(hypervisor) != HypervisorType.None){ c.addCriteria(Criteria.HYPERVISOR, hypervisor); - }else if (hypervisor != null){ + } else if (hypervisor != null){ throw new InvalidParameterValueException("Invalid HypervisorType " + hypervisor); } // ignore these search requests if it's not an admin - if (isAdmin == true) { - c.addCriteria(Criteria.DOMAINID, domainId); + if (isAdmin) { c.addCriteria(Criteria.PODID, cmd.getPodId()); c.addCriteria(Criteria.HOSTID, cmd.getHostId()); c.addCriteria(Criteria.STORAGE_ID, cmd.getStorageId()); @@ -2709,60 +2711,14 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager return searchForUserVMs(c); } - private List recursivelySearchForVms(ListVMsCmd cmd, String path, boolean isAdmin, List domainToSearchWithin, Long accountId) { - - List result = new ArrayList(); - String hypervisor = cmd.getHypervisor(); - for(DomainVO domain : domainToSearchWithin) { - - Criteria c = new Criteria("id", Boolean.TRUE, cmd.getStartIndex(), cmd.getPageSizeVal()); - c.addCriteria(Criteria.KEYWORD, cmd.getKeyword()); - c.addCriteria(Criteria.ID, cmd.getId()); - c.addCriteria(Criteria.NAME, cmd.getInstanceName()); - c.addCriteria(Criteria.STATE, cmd.getState()); - c.addCriteria(Criteria.DATACENTERID, cmd.getZoneId()); - c.addCriteria(Criteria.GROUPID, cmd.getGroupId()); - c.addCriteria(Criteria.FOR_VIRTUAL_NETWORK, cmd.getForVirtualNetwork()); - c.addCriteria(Criteria.NETWORKID, cmd.getNetworkId()); - - if (path != null) { - c.addCriteria(Criteria.PATH, path); - } - - // ignore these search requests if it's not an admin - if (isAdmin == true) { - c.addCriteria(Criteria.DOMAINID, domain.getId()); - c.addCriteria(Criteria.PODID, cmd.getPodId()); - c.addCriteria(Criteria.HOSTID, cmd.getHostId()); - } - - if (HypervisorType.getType(hypervisor) != HypervisorType.None){ - c.addCriteria(Criteria.HYPERVISOR, hypervisor); - }else if (hypervisor != null){ - throw new InvalidParameterValueException("Invalid HypervisorType " + hypervisor); - } - - if (accountId != null) { - c.addCriteria(Criteria.ACCOUNTID, new Object[] {accountId}); - } - c.addCriteria(Criteria.ISADMIN, isAdmin); - - result.addAll(searchForUserVMs(c)); - } - return result; - } @Override public List searchForUserVMs(Criteria c) { Filter searchFilter = new Filter(UserVmVO.class, c.getOrderBy(), c.getAscending(), c.getOffset(), c.getLimit()); SearchBuilder sb = _vmDao.createSearchBuilder(); - - // some criteria matter for generating the join condition Object[] accountIds = (Object[]) c.getCriteria(Criteria.ACCOUNTID); Object domainId = c.getCriteria(Criteria.DOMAINID); - - // get the rest of the criteria Object id = c.getCriteria(Criteria.ID); Object name = c.getCriteria(Criteria.NAME); Object state = c.getCriteria(Criteria.STATE); @@ -2793,21 +2749,14 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager sb.and("hypervisorType", sb.entity().getHypervisorType(), SearchCriteria.Op.EQ); sb.and("hostIdEQ", sb.entity().getHostId(), SearchCriteria.Op.EQ); sb.and("hostIdIN", sb.entity().getHostId(), SearchCriteria.Op.IN); + sb.and("domainId", sb.entity().getDomainId(), SearchCriteria.Op.EQ); - if (domainId != null || path != null) { - // if accountId isn't specified, we can do a domain match for the admin case + if (path != null) { SearchBuilder domainSearch = _domainDao.createSearchBuilder(); - domainSearch.and("id", domainSearch.entity().getId(), SearchCriteria.Op.EQ); domainSearch.and("path", domainSearch.entity().getPath(), SearchCriteria.Op.LIKE); sb.join("domainSearch", domainSearch, sb.entity().getDomainId(), domainSearch.entity().getId(), JoinBuilder.JoinType.INNER); } - if (storageId != null) { - SearchBuilder volumeSearch = _volsDao.createSearchBuilder(); - volumeSearch.and("poolId", volumeSearch.entity().getPoolId(), SearchCriteria.Op.EQ); - sb.join("volumeSearch", volumeSearch, sb.entity().getId(), volumeSearch.entity().getInstanceId(), JoinBuilder.JoinType.INNER); - } - if (groupId != null && (Long)groupId == -1) { SearchBuilder vmSearch = _groupVMMapDao.createSearchBuilder(); vmSearch.and("instanceId", vmSearch.entity().getInstanceId(), SearchCriteria.Op.EQ); @@ -2829,13 +2778,14 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager sb.join("nicSearch", nicSearch, sb.entity().getId(), nicSearch.entity().getInstanceId(), JoinBuilder.JoinType.INNER); } - SearchBuilder accountRemoved = _accountDao.createSearchBuilder(); - accountRemoved.and("accountremoved", accountRemoved.entity().getRemoved(), SearchCriteria.Op.NULL); - sb.join("accountRemoved", accountRemoved, sb.entity().getAccountId(), accountRemoved.entity().getId(), JoinBuilder.JoinType.INNER); + if (storageId != null) { + SearchBuilder volumeSearch = _volsDao.createSearchBuilder(); + volumeSearch.and("poolId", volumeSearch.entity().getPoolId(), SearchCriteria.Op.EQ); + sb.join("volumeSearch", volumeSearch, sb.entity().getId(), volumeSearch.entity().getInstanceId(), JoinBuilder.JoinType.INNER); + } // populate the search criteria with the values passed in - SearchCriteria sc = sb.create(); - + SearchCriteria sc = sb.create(); if (groupId != null && (Long)groupId == -1){ sc.setJoinParameters("vmSearch", "instanceId", (Object)null); } else if (groupId != null ) { @@ -2855,6 +2805,7 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager if (id != null) { sc.setParameters("id", id); } + if (accountIds != null) { if (accountIds.length == 1) { if (accountIds[0] != null) { @@ -2863,18 +2814,16 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager } else { sc.setParameters("accountIdIN", accountIds); } - } else if (domainId != null) { - sc.setJoinParameters("domainSearch", "id", domainId); + } + + if (domainId != null) { + sc.setParameters("domainId", domainId); } if (path != null) { sc.setJoinParameters("domainSearch", "path", path + "%"); } - if (storageId != null) { - sc.setJoinParameters("volumeSearch", "poolId", storageId); - } - if (networkId != null) { sc.setJoinParameters("nicSearch", "networkId", networkId); } @@ -2882,6 +2831,7 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager if (name != null) { sc.setParameters("name", "%" + name + "%"); } + if (state != null) { if (notState != null && (Boolean) notState == true) { sc.setParameters("stateNEQ", state); @@ -2893,6 +2843,8 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager if (hypervisor != null){ sc.setParameters("hypervisorType", hypervisor); } + + //Don't show Destroyed and Expunging vms to the end user if ((isAdmin != null) && ((Boolean) isAdmin != true)) { sc.setParameters("stateNIN", "Destroyed", "Expunging"); } @@ -2900,7 +2852,7 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager if (zone != null) { sc.setParameters("dataCenterId", zone); - if(state == null) { + if (state == null) { sc.setParameters("stateNEQ", "Destroyed"); } } @@ -2929,6 +2881,10 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager } } } + + if (storageId != null) { + sc.setJoinParameters("volumeSearch", "poolId", storageId); + } return _vmDao.search(sc, searchFilter); } diff --git a/setup/db/create-schema.sql b/setup/db/create-schema.sql index 1b727bb089b..73869d5fba1 100755 --- a/setup/db/create-schema.sql +++ b/setup/db/create-schema.sql @@ -967,7 +967,8 @@ CREATE TABLE `cloud`.`domain` ( `removed` datetime COMMENT 'date removed', `state` char(32) NOT NULL default 'Active' COMMENT 'state of the domain', PRIMARY KEY (`id`), - UNIQUE (parent, name, removed) + UNIQUE (parent, name, removed), + INDEX `i_domain__path`(`path`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; CREATE TABLE `cloud`.`account` ( diff --git a/setup/db/schema-222to224.sql b/setup/db/schema-222to224.sql index 56be9cd85e1..c5dc4c636b8 100644 --- a/setup/db/schema-222to224.sql +++ b/setup/db/schema-222to224.sql @@ -57,4 +57,7 @@ ALTER TABLE `cloud`.`cluster` ADD INDEX `i_cluster__allocation_state`(`allocatio ALTER TABLE `cloud`.`host_pod_ref` ADD COLUMN `allocation_state` varchar(32) NOT NULL DEFAULT 'Enabled'; ALTER TABLE `cloud`.`host_pod_ref` ADD INDEX `i_host_pod_ref__allocation_state`(`allocation_state`); ALTER TABLE `cloud`.`host` ADD COLUMN `allocation_state` varchar(32) NOT NULL DEFAULT 'Enabled'; -ALTER TABLE `cloud`.`host` ADD INDEX `i_host__allocation_state`(`allocation_state`); \ No newline at end of file +ALTER TABLE `cloud`.`host` ADD INDEX `i_host__allocation_state`(`allocation_state`); + +ALTER TABLE `cloud`.`domain` ADD INDEX `i_domain__path`(`path`); +