Change AccountManagerImpl.checkAccess to invoke SecurityChecker

interface that takes multiple controlled entities.
This commit is contained in:
Min Chen 2014-04-17 17:53:01 -07:00
parent 4c4cfe5e13
commit da13165743
14 changed files with 133 additions and 169 deletions

View File

@ -104,20 +104,14 @@ public interface AccountService {
RoleType getRoleType(Account account);
void checkAccess(Account account, Domain domain) throws PermissionDeniedException;
void checkAccess(Account caller, Domain domain) throws PermissionDeniedException;
void checkAccess(Account account, AccessType accessType, ControlledEntity... entities) throws PermissionDeniedException;
void checkAccess(Account caller, AccessType accessType, ControlledEntity... entities) throws PermissionDeniedException;
void checkAccess(Account account, AccessType accessType, String apiName, ControlledEntity... entities) throws PermissionDeniedException;
// TODO: the following two interfaces will be deprecated by the above two counterparts when securityChecker implementation is in place
void checkAccess(Account account, AccessType accessType, boolean sameOwner, ControlledEntity... entities) throws PermissionDeniedException;
void checkAccess(Account account, AccessType accessType, boolean sameOwner, String apiName,
ControlledEntity... entities) throws PermissionDeniedException;
void checkAccess(Account caller, AccessType accessType, String apiName, ControlledEntity... entities) throws PermissionDeniedException;
//TO be implemented, to check accessibility for an entity owned by domain
void checkAccess(Account account, AccessType accessType, boolean sameOwner, PartOf... entities) throws PermissionDeniedException;
void checkAccess(Account caller, AccessType accessType, boolean sameOwner, PartOf... entities) throws PermissionDeniedException;
Long finalyzeAccountId(String accountName, Long domainId, Long projectId, boolean enabledOnly);
}

View File

@ -31,10 +31,10 @@ import com.cloud.utils.component.Adapter;
public interface SecurityChecker extends Adapter {
public enum AccessType {
ModifyProject,
OperateEntry,
ListEntry,
UseEntry,
ListEntry
OperateEntry,
ModifyProject,
}
/**

View File

@ -34,8 +34,11 @@ import com.cloud.exception.InvalidParameterValueException;
import com.cloud.exception.NetworkRuleConflictException;
import com.cloud.exception.ResourceUnavailableException;
import com.cloud.network.IpAddress;
import com.cloud.network.vpc.Vpc;
import com.cloud.vm.VirtualMachine;
@APICommand(name = "disableStaticNat", description = "Disables static rule for given ip address", responseObject = SuccessResponse.class,
entityType = {IpAddress.class, VirtualMachine.class, Vpc.class},
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
public class DisableStaticNatCmd extends BaseAsyncCmd {
public static final Logger s_logger = Logger.getLogger(DeletePortForwardingRuleCmd.class.getName());
@ -89,7 +92,7 @@ public class DisableStaticNatCmd extends BaseAsyncCmd {
if (result) {
SuccessResponse response = new SuccessResponse(getCommandName());
this.setResponseObject(response);
setResponseObject(response);
} else {
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to disable static nat");
}

View File

@ -35,12 +35,13 @@ import com.cloud.exception.InvalidParameterValueException;
import com.cloud.exception.NetworkRuleConflictException;
import com.cloud.exception.ResourceUnavailableException;
import com.cloud.network.IpAddress;
import com.cloud.network.vpc.Vpc;
import com.cloud.user.Account;
import com.cloud.uservm.UserVm;
import com.cloud.vm.VirtualMachine;
@APICommand(name = "enableStaticNat", description = "Enables static nat for given ip address", responseObject = SuccessResponse.class,
entityType = {IpAddress.class, VirtualMachine.class},
entityType = {IpAddress.class, VirtualMachine.class, Vpc.class},
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
public class EnableStaticNatCmd extends BaseCmd {
public static final Logger s_logger = Logger.getLogger(CreateIpForwardingRuleCmd.class.getName());

View File

@ -37,7 +37,8 @@ import com.cloud.storage.Volume;
import com.cloud.user.Account;
import com.cloud.vm.VirtualMachine;
@APICommand(name = "attachVolume", description = "Attaches a disk volume to a virtual machine.", responseObject = VolumeResponse.class, responseView = ResponseView.Restricted, entityType = {VirtualMachine.class},
@APICommand(name = "attachVolume", description = "Attaches a disk volume to a virtual machine.", responseObject = VolumeResponse.class, responseView = ResponseView.Restricted, entityType = {
VirtualMachine.class, Volume.class},
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
public class AttachVolumeCmd extends BaseAsyncVolumeCmd {
public static final Logger s_logger = Logger.getLogger(AttachVolumeCmd.class.getName());
@ -52,6 +53,7 @@ public class AttachVolumeCmd extends BaseAsyncVolumeCmd {
+ "* 4 - /dev/xvde" + "* 5 - /dev/xvdf" + "* 6 - /dev/xvdg" + "* 7 - /dev/xvdh" + "* 8 - /dev/xvdi" + "* 9 - /dev/xvdj")
private Long deviceId;
@ACL(accessType = AccessType.OperateEntry)
@Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = VolumeResponse.class, required = true, description = "the ID of the disk volume")
private Long id;

View File

@ -1,3 +1,4 @@
#
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
@ -5,14 +6,72 @@
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
#
# http://www.apache.org/licenses/LICENSE-2.0
#
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
#
# Just here to make the unit test not blow up
# management server clustering parameters, change cluster.node.IP to the machine IP address
# in which the management server(Tomcat) is running
cluster.node.IP=127.0.0.1
cluster.servlet.port=9090
region.id=1
# CloudStack database settings
db.cloud.username=cloud
db.cloud.password=cloud
db.root.password=
db.cloud.host=localhost
db.cloud.port=3306
db.cloud.name=cloud
# CloudStack database tuning parameters
db.cloud.maxActive=250
db.cloud.maxIdle=30
db.cloud.maxWait=10000
db.cloud.autoReconnect=true
db.cloud.validationQuery=SELECT 1
db.cloud.testOnBorrow=true
db.cloud.testWhileIdle=true
db.cloud.timeBetweenEvictionRunsMillis=40000
db.cloud.minEvictableIdleTimeMillis=240000
db.cloud.poolPreparedStatements=false
db.cloud.url.params=prepStmtCacheSize=517&cachePrepStmts=true&prepStmtCacheSqlLimit=4096
# usage database settings
db.usage.username=cloud
db.usage.password=cloud
db.usage.host=localhost
db.usage.port=3306
db.usage.name=cloud_usage
# usage database tuning parameters
db.usage.maxActive=100
db.usage.maxIdle=30
db.usage.maxWait=10000
db.usage.autoReconnect=true
# awsapi database settings
db.awsapi.username=cloud
db.awsapi.password=cloud
db.awsapi.host=localhost
db.awsapi.port=3306
db.awsapi.name=cloudbridge
# Simulator database settings
db.simulator.username=cloud
db.simulator.password=cloud
db.simulator.host=localhost
db.simulator.port=3306
db.simulator.name=simulator
db.simulator.maxActive=250
db.simulator.maxIdle=30
db.simulator.maxWait=10000
db.simulator.autoReconnect=true

View File

@ -95,12 +95,6 @@ public class MockAccountManager extends ManagerBase implements AccountManager {
}
@Override
public void checkAccess(Account arg0, AccessType arg1, boolean arg2, ControlledEntity... arg3) throws PermissionDeniedException {
// TODO Auto-generated method stub
}
@Override
public String[] createApiKeyAndSecretKey(RegisterCmd arg0) {
// TODO Auto-generated method stub
@ -369,11 +363,6 @@ public class MockAccountManager extends ManagerBase implements AccountManager {
}
@Override
public void checkAccess(Account account, AccessType accessType, boolean sameOwner, String apiName,
ControlledEntity... entities) throws PermissionDeniedException {
// TODO Auto-generated method stub
}
@Override
public Long finalyzeAccountId(String accountName, Long domainId, Long projectId, boolean enabledOnly) {

View File

@ -19,6 +19,7 @@ package com.cloud.acl;
import javax.ejb.Local;
import javax.inject.Inject;
import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;
import org.apache.cloudstack.acl.ControlledEntity;
@ -50,6 +51,8 @@ import com.cloud.utils.component.AdapterBase;
@Local(value = SecurityChecker.class)
public class DomainChecker extends AdapterBase implements SecurityChecker {
public static final Logger s_logger = Logger.getLogger(DomainChecker.class);
@Inject
DomainDao _domainDao;
@Inject
@ -101,6 +104,15 @@ public class DomainChecker extends AdapterBase implements SecurityChecker {
@Override
public boolean checkAccess(Account caller, ControlledEntity entity, AccessType accessType)
throws PermissionDeniedException {
if (caller.getId() == Account.ACCOUNT_ID_SYSTEM || _accountService.isRootAdmin(caller.getId())) {
// no need to make permission checks if the system/root admin makes the call
if (s_logger.isTraceEnabled()) {
s_logger.trace("No need to make permission check for System/RootAdmin account, returning true");
}
return true;
}
if (entity instanceof VirtualMachineTemplate) {
VirtualMachineTemplate template = (VirtualMachineTemplate)entity;

View File

@ -23,10 +23,6 @@ import javax.inject.Inject;
import org.apache.log4j.Logger;
import org.apache.cloudstack.acl.ControlledEntity;
import org.apache.cloudstack.acl.InfrastructureEntity;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
import org.apache.cloudstack.api.APICommand;
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.api.BaseAsyncCmd;
import org.apache.cloudstack.api.BaseAsyncCreateCmd;
@ -41,7 +37,6 @@ import com.cloud.api.dispatch.DispatchChain;
import com.cloud.api.dispatch.DispatchChainFactory;
import com.cloud.api.dispatch.DispatchTask;
import com.cloud.event.EventTypes;
import com.cloud.user.Account;
import com.cloud.user.AccountManager;
import com.cloud.utils.ReflectUtil;
import com.cloud.vm.VirtualMachine;
@ -83,23 +78,6 @@ public class ApiDispatcher {
CallContext.current().setEventDisplayEnabled(cmd.isDisplayResourceEnabled());
}
private void doAccessChecks(BaseCmd cmd, Map<Object, AccessType> entitiesToAccess) {
Account caller = CallContext.current().getCallingAccount();
APICommand commandAnnotation = cmd.getClass().getAnnotation(APICommand.class);
String apiName = commandAnnotation != null ? commandAnnotation.name() : null;
if (!entitiesToAccess.isEmpty()) {
for (Object entity : entitiesToAccess.keySet()) {
if (entity instanceof ControlledEntity) {
_accountMgr.checkAccess(caller, entitiesToAccess.get(entity), false, apiName, (ControlledEntity) entity);
} else if (entity instanceof InfrastructureEntity) {
//FIXME: Move this code in adapter, remove code from Account manager
}
}
}
}
public void dispatch(final BaseCmd cmd, final Map<String, String> params, final boolean execute) throws Exception {
// Let the chain of responsibility dispatch gradually
standardDispatchChain.dispatch(new DispatchTask(cmd, params));

View File

@ -231,35 +231,39 @@ public class ParamProcessWorker implements DispatchWorker {
private void doAccessChecks(final BaseCmd cmd, final Map<Object, AccessType> entitiesToAccess) {
Account caller = CallContext.current().getCallingAccount();
Account owner = _accountMgr.getAccount(cmd.getEntityOwnerId());
if (owner == null) {
owner = caller;
}
APICommand commandAnnotation = cmd.getClass().getAnnotation(APICommand.class);
String apiName = commandAnnotation != null ? commandAnnotation.name() : null;
if (!entitiesToAccess.isEmpty()) {
List<ControlledEntity> entitiesToOperate = new ArrayList<ControlledEntity>();
for (Object entity : entitiesToAccess.keySet()) {
if (entity instanceof ControlledEntity) {
if (AccessType.OperateEntry == entitiesToAccess.get(entity)) {
entitiesToOperate.add((ControlledEntity) entity);
} else {
_accountMgr.checkAccess(caller, entitiesToAccess.get(entity), apiName,
_accountMgr.checkAccess(owner, entitiesToAccess.get(entity), apiName,
(ControlledEntity) entity);
}
} else if (entity instanceof InfrastructureEntity) {
if (entity instanceof DataCenter) {
checkZoneAccess(caller, (DataCenter) entity);
checkZoneAccess(owner, (DataCenter)entity);
} else if (entity instanceof ServiceOffering) {
checkServiceOfferingAccess(caller, (ServiceOffering) entity);
checkServiceOfferingAccess(owner, (ServiceOffering)entity);
} else if (entity instanceof DiskOffering) {
checkDiskOfferingAccess(caller, (DiskOffering) entity);
checkDiskOfferingAccess(owner, (DiskOffering)entity);
}
}
}
if (!entitiesToOperate.isEmpty()) {
_accountMgr.checkAccess(caller, AccessType.OperateEntry, false, apiName,
_accountMgr.checkAccess(owner, AccessType.OperateEntry, apiName,
entitiesToOperate.toArray(new ControlledEntity[entitiesToOperate.size()]));
}

View File

@ -1140,6 +1140,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
}
// permission check
// TODO: remove this if we can annotate volume parameter in createVolumeCmd since this routine is used there as well.
_accountMgr.checkAccess(caller, AccessType.OperateEntry, volumeToAttach, vm);
if (!(Volume.State.Allocated.equals(volumeToAttach.getState()) || Volume.State.Ready.equals(volumeToAttach.getState()) || Volume.State.Uploaded.equals(volumeToAttach

View File

@ -46,7 +46,6 @@ import org.apache.cloudstack.acl.QuerySelector;
import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.acl.SecurityChecker;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
import org.apache.cloudstack.affinity.AffinityGroup;
import org.apache.cloudstack.affinity.dao.AffinityGroupDao;
import org.apache.cloudstack.api.InternalIdentity;
import org.apache.cloudstack.api.command.admin.account.UpdateAccountCmd;
@ -61,7 +60,6 @@ import org.apache.cloudstack.framework.messagebus.PublishScope;
import org.apache.cloudstack.managed.context.ManagedContextRunnable;
import org.apache.cloudstack.region.gslb.GlobalLoadBalancerRuleDao;
import com.cloud.api.ApiDBUtils;
import com.cloud.configuration.Config;
import com.cloud.configuration.ConfigurationManager;
import com.cloud.configuration.Resource.ResourceOwnerType;
@ -91,7 +89,6 @@ import com.cloud.exception.PermissionDeniedException;
import com.cloud.exception.ResourceUnavailableException;
import com.cloud.network.IpAddress;
import com.cloud.network.IpAddressManager;
import com.cloud.network.Network;
import com.cloud.network.VpnUserVO;
import com.cloud.network.as.AutoScaleManager;
import com.cloud.network.dao.AccountGuestVlanMapDao;
@ -126,7 +123,6 @@ import com.cloud.storage.dao.VMTemplateDao;
import com.cloud.storage.dao.VolumeDao;
import com.cloud.storage.snapshot.SnapshotManager;
import com.cloud.template.TemplateManager;
import com.cloud.template.VirtualMachineTemplate;
import com.cloud.user.Account.State;
import com.cloud.user.dao.AccountDao;
import com.cloud.user.dao.UserAccountDao;
@ -443,7 +439,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
}
@Override
public void checkAccess(Account account, AccessType accessType, boolean sameOwner, PartOf... entities) throws PermissionDeniedException {
public void checkAccess(Account caller, AccessType accessType, boolean sameOwner, PartOf... entities) throws PermissionDeniedException {
// TODO Auto-generated method stub
//TO BE IMPLEMENTED
@ -451,103 +447,34 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
}
@Override
public void checkAccess(Account account, AccessType accessType, ControlledEntity... entities) throws PermissionDeniedException {
// TODO this will eventually deprecate below sameOwner check interface.
// TO BE IMPLEMENTED when multiple controlled entity support interface is added into SecurityChecker
checkAccess(account, accessType, false, entities);
public void checkAccess(Account caller, AccessType accessType, ControlledEntity... entities) throws PermissionDeniedException {
checkAccess(caller, accessType, null, entities);
}
@Override
public void checkAccess(Account account, AccessType accessType, String apiName, ControlledEntity... entities) throws PermissionDeniedException {
// TODO this will eventually deprecate below sameOwner check interface.
// TO BE IMPLEMENTED when multiple controlled entity support interface is added into SecurityChecker
checkAccess(account, accessType, false, apiName, entities);
}
@Override
public void checkAccess(Account caller, AccessType accessType, boolean sameOwner, ControlledEntity... entities) {
checkAccess(caller, accessType, sameOwner, null, entities);
}
@Override
public void checkAccess(Account caller, AccessType accessType, boolean sameOwner, String apiName, ControlledEntity... entities) {
//check for the same owner
Long ownerId = null;
ControlledEntity prevEntity = null;
if (sameOwner) {
for (ControlledEntity entity : entities) {
if (sameOwner) {
if (ownerId == null) {
ownerId = entity.getAccountId();
} else if (ownerId.longValue() != entity.getAccountId()) {
throw new PermissionDeniedException("Entity " + entity + " and entity " + prevEntity + " belong to different accounts");
}
prevEntity = entity;
public void checkAccess(Account caller, AccessType accessType, String apiName, ControlledEntity... entities) throws PermissionDeniedException {
boolean granted = false;
// construct entities identification string
StringBuffer entityBuf = new StringBuffer("{");
for (ControlledEntity ent : entities) {
entityBuf.append(ent.toString());
}
entityBuf.append("}");
String entityStr = entityBuf.toString();
for (SecurityChecker checker : _securityCheckers) {
if (checker.checkAccess(caller, accessType, apiName, entities)) {
if (s_logger.isDebugEnabled()) {
s_logger.debug("Access to " + entityStr + " granted to " + caller + " by " + checker.getName());
}
granted = true;
break;
}
}
if (caller.getId() == Account.ACCOUNT_ID_SYSTEM || isRootAdmin(caller.getId())) {
// no need to make permission checks if the system/root admin makes the call
if (s_logger.isTraceEnabled()) {
s_logger.trace("No need to make permission check for System/RootAdmin account, returning true");
}
return;
if (!granted) {
assert false : "How can all of the security checkers pass on checking this check: " + entityStr;
throw new PermissionDeniedException("There's no way to confirm " + caller + " has access to " + entityStr);
}
HashMap<Long, List<ControlledEntity>> domains = new HashMap<Long, List<ControlledEntity>>();
for (ControlledEntity entity : entities) {
long domainId = entity.getDomainId();
if (entity.getAccountId() != -1 && domainId == -1) { // If account exists domainId should too so calculate
// it. This condition might be hit for templates or entities which miss domainId in their tables
Account account = ApiDBUtils.findAccountById(entity.getAccountId());
domainId = account != null ? account.getDomainId() : -1;
}
if (entity.getAccountId() != -1 && domainId != -1 && !(entity instanceof VirtualMachineTemplate) &&
!(entity instanceof Network && accessType != null && accessType == AccessType.UseEntry) && !(entity instanceof AffinityGroup)) {
List<ControlledEntity> toBeChecked = domains.get(entity.getDomainId());
// for templates, we don't have to do cross domains check
if (toBeChecked == null) {
toBeChecked = new ArrayList<ControlledEntity>();
domains.put(domainId, toBeChecked);
}
toBeChecked.add(entity);
}
boolean granted = false;
for (SecurityChecker checker : _securityCheckers) {
if (checker.checkAccess(caller, entity, accessType, apiName)) {
if (s_logger.isDebugEnabled()) {
s_logger.debug("Access to " + entity + " granted to " + caller + " by " + checker.getName());
}
granted = true;
break;
}
}
if (!granted) {
assert false : "How can all of the security checkers pass on checking this check: " + entity;
throw new PermissionDeniedException("There's no way to confirm " + caller + " has access to " + entity);
}
}
for (Map.Entry<Long, List<ControlledEntity>> domain : domains.entrySet()) {
for (SecurityChecker checker : _securityCheckers) {
Domain d = _domainMgr.getDomain(domain.getKey());
if (d == null || d.getRemoved() != null) {
throw new PermissionDeniedException("Domain is not found.", caller, domain.getValue());
}
try {
checker.checkAccess(caller, d);
} catch (PermissionDeniedException e) {
e.addDetails(caller, domain.getValue());
throw e;
}
}
}
// check that resources belong to the same account
}
@Override

View File

@ -219,10 +219,6 @@ public class MockAccountManagerImpl extends ManagerBase implements Manager, Acco
return null;
}
@Override
public void checkAccess(Account account, AccessType accessType, boolean sameOwner, ControlledEntity... entities) throws PermissionDeniedException {
// TODO Auto-generated method stub
}
@Override
public void checkAccess(Account account, AccessType accessType, boolean sameOwner, PartOf... entities) throws PermissionDeniedException {
@ -344,11 +340,6 @@ public class MockAccountManagerImpl extends ManagerBase implements Manager, Acco
return null;
}
@Override
public void checkAccess(Account account, AccessType accessType, boolean sameOwner, String apiName,
ControlledEntity... entities) throws PermissionDeniedException {
// TODO Auto-generated method stub
}
@Override
public Long finalyzeAccountId(String accountName, Long domainId, Long projectId, boolean enabledOnly) {

View File

@ -27,7 +27,6 @@ import org.apache.log4j.Logger;
import org.apache.cloudstack.acl.ControlledEntity;
import org.apache.cloudstack.acl.PermissionScope;
import org.apache.cloudstack.acl.SecurityChecker;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
import org.apache.cloudstack.api.InternalIdentity;
import org.apache.cloudstack.iam.api.IAMGroup;
import org.apache.cloudstack.iam.api.IAMPolicy;
@ -168,13 +167,15 @@ public class RoleBasedEntityAccessChecker extends DomainChecker implements Secur
boolean otherEntitiesAccess = true;
for (ControlledEntity otherEntity : entities) {
if (otherEntity.getAccountId() == caller.getAccountId()
|| (checkAccess(caller, otherEntity, accessType, action) && otherEntity.getAccountId() == entity
.getAccountId())) {
continue;
} else {
otherEntitiesAccess = false;
break;
if (otherEntity != entity) {
if (otherEntity.getAccountId() == caller.getAccountId()
|| (checkAccess(caller, otherEntity, accessType, action) && otherEntity.getAccountId() == entity
.getAccountId())) {
continue;
} else {
otherEntitiesAccess = false;
break;
}
}
}
@ -225,6 +226,8 @@ public class RoleBasedEntityAccessChecker extends DomainChecker implements Secur
if (_domainDao.isChildDomain(caller.getDomainId(), entity.getDomainId())) {
return true;
}
} else if (scope.equals(PermissionScope.ALL.name())) {
return true;
}
}
return false;