diff --git a/api/src/com/cloud/api/commands/AttachIsoCmd.java b/api/src/com/cloud/api/commands/AttachIsoCmd.java index 6e0d74a58da..4cb34d1536b 100755 --- a/api/src/com/cloud/api/commands/AttachIsoCmd.java +++ b/api/src/com/cloud/api/commands/AttachIsoCmd.java @@ -27,8 +27,7 @@ import com.cloud.api.Parameter; import com.cloud.api.ServerApiException; import com.cloud.api.response.UserVmResponse; import com.cloud.event.EventTypes; -import com.cloud.template.VirtualMachineTemplate; -import com.cloud.user.Account; +import com.cloud.exception.InvalidParameterValueException; import com.cloud.uservm.UserVm; @Implementation(description="Attaches an ISO to a virtual machine.", responseObject=UserVmResponse.class) @@ -72,11 +71,12 @@ public class AttachIsoCmd extends BaseAsyncCmd { @Override public long getEntityOwnerId() { - VirtualMachineTemplate iso = _responseGenerator.findTemplateById(getId()); - if (iso == null) { - return Account.ACCOUNT_ID_SYSTEM; // bad id given, parent this command to SYSTEM so ERROR events are tracked - } - return iso.getAccountId(); + UserVm vm = _entityMgr.findById(UserVm.class, getVirtualMachineId()); + if (vm == null) { + throw new InvalidParameterValueException("Unable to find virtual machine by id " + getVirtualMachineId()); + } + + return vm.getAccountId(); } @Override diff --git a/api/src/com/cloud/api/commands/DetachIsoCmd.java b/api/src/com/cloud/api/commands/DetachIsoCmd.java index 7f69f6c1841..9cf6b9e9a04 100755 --- a/api/src/com/cloud/api/commands/DetachIsoCmd.java +++ b/api/src/com/cloud/api/commands/DetachIsoCmd.java @@ -27,7 +27,7 @@ import com.cloud.api.Parameter; import com.cloud.api.ServerApiException; import com.cloud.api.response.UserVmResponse; import com.cloud.event.EventTypes; -import com.cloud.user.Account; +import com.cloud.exception.InvalidParameterValueException; import com.cloud.uservm.UserVm; @Implementation(description="Detaches any ISO file (if any) currently attached to a virtual machine.", responseObject=UserVmResponse.class) @@ -65,9 +65,9 @@ public class DetachIsoCmd extends BaseAsyncCmd { UserVm vm = _entityMgr.findById(UserVm.class, getVirtualMachineId()); if (vm != null) { return vm.getAccountId(); - } - - return Account.ACCOUNT_ID_SYSTEM; // no account info given, parent this command to SYSTEM so ERROR events are tracked + } else { + throw new InvalidParameterValueException("Unable to find vm by id " + getVirtualMachineId()); + } } @Override diff --git a/api/src/com/cloud/api/commands/RegisterIsoCmd.java b/api/src/com/cloud/api/commands/RegisterIsoCmd.java index 8569d3e4763..fdc9cbeb755 100755 --- a/api/src/com/cloud/api/commands/RegisterIsoCmd.java +++ b/api/src/com/cloud/api/commands/RegisterIsoCmd.java @@ -26,6 +26,7 @@ import com.cloud.api.Parameter; import com.cloud.api.ServerApiException; import com.cloud.api.response.ListResponse; import com.cloud.api.response.TemplateResponse; +import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.ResourceAllocationException; import com.cloud.template.VirtualMachineTemplate; import com.cloud.user.Account; @@ -127,20 +128,18 @@ public class RegisterIsoCmd extends BaseCmd { @Override public long getEntityOwnerId() { Account account = UserContext.current().getCaller(); - if ((account == null) || isAdmin(account.getType())) { + if (isAdmin(account.getType())) { if ((domainId != null) && (accountName != null)) { Account userAccount = _responseGenerator.findAccountByNameDomain(accountName, domainId); if (userAccount != null) { return userAccount.getId(); + } else { + throw new InvalidParameterValueException("Unable to find account by name " + getAccountName() + " in domain " + getDomainId()); } } } - - if (account != null) { - return account.getId(); - } - - return Account.ACCOUNT_ID_SYSTEM; // no account info given, parent this command to SYSTEM so ERROR events are tracked + + return account.getId(); } @Override diff --git a/api/src/com/cloud/api/commands/RegisterTemplateCmd.java b/api/src/com/cloud/api/commands/RegisterTemplateCmd.java index 8f49fca75f2..4c9882a5f3a 100755 --- a/api/src/com/cloud/api/commands/RegisterTemplateCmd.java +++ b/api/src/com/cloud/api/commands/RegisterTemplateCmd.java @@ -26,10 +26,10 @@ import com.cloud.api.BaseCmd; import com.cloud.api.Implementation; import com.cloud.api.Parameter; import com.cloud.api.ServerApiException; -import com.cloud.api.BaseCmd.CommandType; import com.cloud.api.response.ListResponse; import com.cloud.api.response.TemplateResponse; import com.cloud.async.AsyncJob; +import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.ResourceAllocationException; import com.cloud.template.VirtualMachineTemplate; import com.cloud.user.Account; @@ -177,20 +177,18 @@ public class RegisterTemplateCmd extends BaseCmd { @Override public long getEntityOwnerId() { Account account = UserContext.current().getCaller(); - if ((account == null) || isAdmin(account.getType())) { + if (isAdmin(account.getType())) { if ((domainId != null) && (accountName != null)) { Account userAccount = _responseGenerator.findAccountByNameDomain(accountName, domainId); if (userAccount != null) { return userAccount.getId(); + } else { + throw new InvalidParameterValueException("Unable to find account by name " + getAccountName() + " in domain " + getDomainId()); } } } - - if (account != null) { - return account.getId(); - } - - return Account.ACCOUNT_ID_SYSTEM; // no account info given, parent this command to SYSTEM so ERROR events are tracked + + return account.getId(); } @Override diff --git a/api/src/com/cloud/user/AccountService.java b/api/src/com/cloud/user/AccountService.java index 1cb936e0cff..d9db18c314a 100644 --- a/api/src/com/cloud/user/AccountService.java +++ b/api/src/com/cloud/user/AccountService.java @@ -155,6 +155,8 @@ public interface AccountService { User getActiveUser(long userId); + User getUser(long userId); + Domain getDomain(long id); boolean isRootAdmin(short accountType); diff --git a/server/src/com/cloud/async/AsyncJobManagerImpl.java b/server/src/com/cloud/async/AsyncJobManagerImpl.java index 75dde9eae46..764fa4f2c3f 100644 --- a/server/src/com/cloud/async/AsyncJobManagerImpl.java +++ b/server/src/com/cloud/async/AsyncJobManagerImpl.java @@ -53,6 +53,7 @@ import com.cloud.exception.PermissionDeniedException; import com.cloud.maid.StackMaid; import com.cloud.user.Account; import com.cloud.user.AccountManager; +import com.cloud.user.User; import com.cloud.user.UserContext; import com.cloud.user.dao.AccountDao; import com.cloud.utils.DateUtil; @@ -281,7 +282,9 @@ public class AsyncJobManagerImpl implements AsyncJobManager, ClusterManagerListe if (job == null) { throw new InvalidParameterValueException("Unable to find a job by id " + cmd.getId()); } - Account jobOwner = _accountMgr.getAccount(job.getAccountId()); + + User userJobOwner = _accountMgr.getUser(job.getUserId()); + Account jobOwner = _accountMgr.getAccount(userJobOwner.getAccountId()); //check permissions if (caller.getType() == Account.ACCOUNT_TYPE_NORMAL) { diff --git a/server/src/com/cloud/template/TemplateManagerImpl.java b/server/src/com/cloud/template/TemplateManagerImpl.java index d54131f480c..47bd209ac6e 100755 --- a/server/src/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/com/cloud/template/TemplateManagerImpl.java @@ -1203,14 +1203,14 @@ public class TemplateManagerImpl implements TemplateManager, Manager, TemplateSe @Override public boolean attachIso(AttachIsoCmd cmd) { - Account account = UserContext.current().getCaller(); + Account caller = UserContext.current().getCaller(); Long userId = UserContext.current().getCallerUserId(); Long vmId = cmd.getVirtualMachineId(); Long isoId = cmd.getId(); // Verify input parameters - UserVmVO vmInstanceCheck = _userVmDao.findById(vmId); - if (vmInstanceCheck == null) { + UserVmVO vm = _userVmDao.findById(vmId); + if (vm == null) { throw new InvalidParameterValueException("Unable to find a virtual machine with id " + vmId); } @@ -1219,16 +1219,16 @@ public class TemplateManagerImpl implements TemplateManager, Manager, TemplateSe throw new InvalidParameterValueException("Unable to find an ISO with id " + isoId); } - State vmState = vmInstanceCheck.getState(); + State vmState = vm.getState(); if (vmState != State.Running && vmState != State.Stopped) { throw new InvalidParameterValueException("Please specify a VM that is either Stopped or Running."); } String errMsg = "Unable to attach ISO" + isoId + "to virtual machine " + vmId; - userId = accountAndUserValidation(account, userId, vmInstanceCheck, iso, errMsg); + userId = accountAndUserValidation(caller, userId, vm, iso, errMsg); - if ("xen-pv-drv-iso".equals(iso.getDisplayText()) && vmInstanceCheck.getHypervisorType() != Hypervisor.HypervisorType.XenServer){ - throw new InvalidParameterValueException("Cannot attach Xenserver PV drivers to incompatible hypervisor " + vmInstanceCheck.getHypervisorType()); + if ("xen-pv-drv-iso".equals(iso.getDisplayText()) && vm.getHypervisorType() != Hypervisor.HypervisorType.XenServer){ + throw new InvalidParameterValueException("Cannot attach Xenserver PV drivers to incompatible hypervisor " + vm.getHypervisorType()); } return attachISOToVM(vmId, userId, isoId, true); @@ -1250,36 +1250,29 @@ public class TemplateManagerImpl implements TemplateManager, Manager, TemplateSe return success; } - private Long accountAndUserValidation(Account account, Long userId, UserVmVO vmInstanceCheck, VMTemplateVO template, String msg) throws PermissionDeniedException{ + private Long accountAndUserValidation(Account caller, Long userId, UserVmVO userVm, VMTemplateVO template, String msg) throws PermissionDeniedException{ - if (account != null) { - if (!isAdmin(account.getType())) { - if ((vmInstanceCheck != null) && (account.getId() != vmInstanceCheck.getAccountId())) { + if (caller != null) { + if (!isAdmin(caller.getType())) { + if ((userVm != null) && (caller.getId() != userVm.getAccountId())) { throw new PermissionDeniedException(msg + ". Permission denied."); } - if ((template != null) && (!template.isPublicTemplate() && (account.getId() != template.getAccountId()) && (template.getTemplateType() != TemplateType.PERHOST))) { + if ((template != null) && (!template.isPublicTemplate() && (caller.getId() != template.getAccountId()) && (template.getTemplateType() != TemplateType.PERHOST))) { throw new PermissionDeniedException(msg + ". Permission denied."); } } else { - if ((vmInstanceCheck != null) && !_domainDao.isChildDomain(account.getDomainId(), vmInstanceCheck.getDomainId())) { - throw new PermissionDeniedException(msg + ". Permission denied."); + if (userVm != null) { + _accountMgr.checkAccess(caller, userVm); } - // FIXME: if template/ISO owner is null we probably need to throw some kind of exception - if (template != null) { + if (template != null && !template.isPublicTemplate()) { Account templateOwner = _accountDao.findById(template.getAccountId()); - if ((templateOwner != null) && !_domainDao.isChildDomain(account.getDomainId(), templateOwner.getDomainId())) { - throw new PermissionDeniedException(msg + ". Permission denied."); - } + _accountMgr.checkAccess(caller, templateOwner); } } } - // If command is executed via 8096 port, set userId to the id of System account (1) - if (userId == null) { - userId = new Long(1); - } return userId; } diff --git a/server/src/com/cloud/user/AccountManagerImpl.java b/server/src/com/cloud/user/AccountManagerImpl.java index d5e524d8d6e..8b5ffd6d96b 100755 --- a/server/src/com/cloud/user/AccountManagerImpl.java +++ b/server/src/com/cloud/user/AccountManagerImpl.java @@ -1695,6 +1695,11 @@ public class AccountManagerImpl implements AccountManager, AccountService, Manag public User getActiveUser(long userId) { return _userDao.findById(userId); } + + @Override + public User getUser(long userId) { + return _userDao.findByIdIncludingRemoved(userId); + } @Override public Domain getDomain(long domainId) {