bug 9898: fixed attachIso by domain admin - no need to make permission check when iso is public

status 9898: resolved fixed
This commit is contained in:
alena 2011-05-17 11:08:51 -07:00
parent b0cc754c85
commit 9f65b08c7f
8 changed files with 50 additions and 50 deletions

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -155,6 +155,8 @@ public interface AccountService {
User getActiveUser(long userId);
User getUser(long userId);
Domain getDomain(long id);
boolean isRootAdmin(short accountType);

View File

@ -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) {

View File

@ -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;
}

View File

@ -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) {