SecurityChecker can accept multiple ControlledEntity
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/897e0d3a Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/897e0d3a Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/897e0d3a Branch: refs/heads/marvin Commit: 897e0d3abe38d8dde60387599588e2fce6c16124 Parents: ecbaa11 Author: Prachi Damle <[email protected]> Authored: Tue Apr 1 11:25:23 2014 -0700 Committer: Min Chen <[email protected]> Committed: Fri Apr 4 16:38:29 2014 -0700 ---------------------------------------------------------------------- .../apache/cloudstack/acl/SecurityChecker.java | 20 ++++ server/src/com/cloud/acl/DomainChecker.java | 13 +++ .../cloud/api/dispatch/ParamProcessWorker.java | 109 +++++++++++++++++-- .../iam/RoleBasedAPIAccessChecker.java | 1 - .../iam/RoleBasedEntityAccessChecker.java | 66 +++++++++-- 5 files changed, 186 insertions(+), 23 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/897e0d3a/api/src/org/apache/cloudstack/acl/SecurityChecker.java ---------------------------------------------------------------------- diff --git a/api/src/org/apache/cloudstack/acl/SecurityChecker.java b/api/src/org/apache/cloudstack/acl/SecurityChecker.java index 8ca34d0..0b29fb4 100644 --- a/api/src/org/apache/cloudstack/acl/SecurityChecker.java +++ b/api/src/org/apache/cloudstack/acl/SecurityChecker.java @@ -100,6 +100,26 @@ public interface SecurityChecker extends Adapter { */ boolean checkAccess(Account caller, ControlledEntity entity, AccessType accessType, String action) throws PermissionDeniedException; + /** + * Checks if the account can access multiple objects. + * + * @param caller + * account to check against. + * @param entities + * objects that the account is trying to access. + * @param accessType + * TODO + * @param action + * name of the API + * @return true if access allowed. false if this adapter cannot provide + * permission. + * @throws PermissionDeniedException + * if this adapter is suppose to authenticate ownership and the + * check failed. + */ + boolean checkAccess(Account caller, AccessType accessType, String action, ControlledEntity... entities) + throws PermissionDeniedException; + /** * Checks if the user belongs to an account that can access the object. http://git-wip-us.apache.org/repos/asf/cloudstack/blob/897e0d3a/server/src/com/cloud/acl/DomainChecker.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/acl/DomainChecker.java b/server/src/com/cloud/acl/DomainChecker.java index ea129f7..14976cc 100755 --- a/server/src/com/cloud/acl/DomainChecker.java +++ b/server/src/com/cloud/acl/DomainChecker.java @@ -341,4 +341,17 @@ public class DomainChecker extends AdapterBase implements SecurityChecker { } return checkAccess(caller, entity, accessType); } + + @Override + public boolean checkAccess(Account caller, AccessType accessType, String action, ControlledEntity... entities) + throws PermissionDeniedException { + + // returns true only if access to all entities is granted + for (ControlledEntity entity : entities) { + if (!checkAccess(caller, entity, accessType, action)) { + return false; + } + } + return true; + } } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/897e0d3a/server/src/com/cloud/api/dispatch/ParamProcessWorker.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/api/dispatch/ParamProcessWorker.java b/server/src/com/cloud/api/dispatch/ParamProcessWorker.java index e9bdd8b..3f1b47f 100644 --- a/server/src/com/cloud/api/dispatch/ParamProcessWorker.java +++ b/server/src/com/cloud/api/dispatch/ParamProcessWorker.java @@ -37,8 +37,10 @@ import org.apache.log4j.Logger; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.InfrastructureEntity; +import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.api.ACL; +import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiErrorCode; import org.apache.cloudstack.api.BaseAsyncCreateCmd; import org.apache.cloudstack.api.BaseCmd; @@ -54,7 +56,12 @@ import org.apache.cloudstack.api.command.user.event.DeleteEventsCmd; import org.apache.cloudstack.api.command.user.event.ListEventsCmd; import org.apache.cloudstack.context.CallContext; +import com.cloud.configuration.ConfigurationManager; +import com.cloud.dc.DataCenter; import com.cloud.exception.InvalidParameterValueException; +import com.cloud.exception.PermissionDeniedException; +import com.cloud.offering.DiskOffering; +import com.cloud.offering.ServiceOffering; import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.utils.DateUtil; @@ -71,6 +78,17 @@ public class ParamProcessWorker implements DispatchWorker { @Inject protected EntityManager _entityMgr; + List<SecurityChecker> _secChecker; + + public List<SecurityChecker> getSecChecker() { + return _secChecker; + } + + @Inject + public void setSecChecker(List<SecurityChecker> secChecker) { + _secChecker = secChecker; + } + @Override public void handle(final DispatchTask task) { processParameters(task.getCmd(), task.getParams()); @@ -214,27 +232,96 @@ public class ParamProcessWorker implements DispatchWorker { private void doAccessChecks(final BaseCmd cmd, final Map<Object, AccessType> entitiesToAccess) { - final Account caller = CallContext.current().getCallingAccount(); - final Account owner = _accountMgr.getActiveAccountById(cmd.getEntityOwnerId()); + Account caller = CallContext.current().getCallingAccount(); - if (cmd instanceof BaseAsyncCreateCmd) { - //check that caller can access the owner account. - _accountMgr.checkAccess(caller, null, true, owner); - } + APICommand commandAnnotation = cmd.getClass().getAnnotation(APICommand.class); + String apiName = commandAnnotation != null ? commandAnnotation.name() : null; if (!entitiesToAccess.isEmpty()) { - //check that caller can access the owner account. - _accountMgr.checkAccess(caller, null, true, owner); - for (final Object entity : entitiesToAccess.keySet()) { + List<ControlledEntity> entitiesToOperate = new ArrayList<ControlledEntity>(); + + for (Object entity : entitiesToAccess.keySet()) { if (entity instanceof ControlledEntity) { - _accountMgr.checkAccess(caller, entitiesToAccess.get(entity), true, (ControlledEntity)entity); + + if (AccessType.OperateEntry == entitiesToAccess.get(entity)) { + entitiesToOperate.add((ControlledEntity) entity); + } else { + _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 + if (entity instanceof DataCenter) { + checkZoneAccess(caller, (DataCenter) entity); + } else if (entity instanceof ServiceOffering) { + checkServiceOfferingAccess(caller, (ServiceOffering) entity); + } else if (entity instanceof DiskOffering) { + checkDiskOfferingAccess(caller, (DiskOffering) entity); + } } } + + if (!entitiesToOperate.isEmpty()) { + _accountMgr.checkAccess(caller, AccessType.OperateEntry, false, apiName, + (ControlledEntity[]) entitiesToOperate.toArray()); + } + } } + private void checkDiskOfferingAccess(Account caller, DiskOffering dof) { + for (SecurityChecker checker : _secChecker) { + if (checker.checkAccess(caller, dof)) { + if (s_logger.isDebugEnabled()) { + s_logger.debug("Access granted to " + caller + " to disk offering:" + dof.getId() + " by " + + checker.getName()); + } + return; + } else { + throw new PermissionDeniedException("Access denied to " + caller + " by " + checker.getName()); + } + } + + assert false : "How can all of the security checkers pass on checking this caller?"; + throw new PermissionDeniedException("There's no way to confirm " + caller + " has access to disk offering:" + + dof.getId()); + } + + private void checkServiceOfferingAccess(Account caller, ServiceOffering sof) { + for (SecurityChecker checker : _secChecker) { + if (checker.checkAccess(caller, sof)) { + if (s_logger.isDebugEnabled()) { + s_logger.debug("Access granted to " + caller + " to service offering:" + sof.getId() + " by " + + checker.getName()); + } + return; + } else { + throw new PermissionDeniedException("Access denied to " + caller + " by " + checker.getName()); + } + } + + assert false : "How can all of the security checkers pass on checking this caller?"; + throw new PermissionDeniedException("There's no way to confirm " + caller + " has access to service offering:" + + sof.getId()); + } + + private void checkZoneAccess(Account caller, DataCenter zone) { + for (SecurityChecker checker : _secChecker) { + if (checker.checkAccess(caller, zone)) { + if (s_logger.isDebugEnabled()) { + s_logger.debug("Access granted to " + caller + " to zone:" + zone.getId() + " by " + + checker.getName()); + } + return; + } else { + throw new PermissionDeniedException("Access denied to " + caller + " by " + checker.getName() + + " for zone " + zone.getId()); + } + } + + assert false : "How can all of the security checkers pass on checking this caller?"; + throw new PermissionDeniedException("There's no way to confirm " + caller + " has access to zone:" + + zone.getId()); + } @SuppressWarnings({"unchecked", "rawtypes"}) private void setFieldValue(final Field field, final BaseCmd cmdObj, final Object paramObj, final Parameter annotation) throws IllegalArgumentException, ParseException { http://git-wip-us.apache.org/repos/asf/cloudstack/blob/897e0d3a/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedAPIAccessChecker.java ---------------------------------------------------------------------- diff --git a/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedAPIAccessChecker.java b/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedAPIAccessChecker.java index 1afe5e8..9964d48 100644 --- a/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedAPIAccessChecker.java +++ b/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedAPIAccessChecker.java @@ -216,7 +216,6 @@ public class RoleBasedAPIAccessChecker extends AdapterBase implements APIChecker } private void addDefaultAclPolicyPermission(String apiName, Class<?> cmdClass, RoleType role) { - AccessType accessType = null; Class<?>[] entityTypes = null; if (cmdClass != null) { http://git-wip-us.apache.org/repos/asf/cloudstack/blob/897e0d3a/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityAccessChecker.java ---------------------------------------------------------------------- diff --git a/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityAccessChecker.java b/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityAccessChecker.java index bcc483f..295a106 100644 --- a/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityAccessChecker.java +++ b/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityAccessChecker.java @@ -27,6 +27,7 @@ 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; @@ -107,14 +108,22 @@ public class RoleBasedEntityAccessChecker extends DomainChecker implements Secur permissions = _iamSrv.listPolicyPermissionByActionAndEntity(policy.getId(), action, entityType); if (permissions.isEmpty()) { if (accessType != null) { - permissions.addAll(_iamSrv.listPolicyPermissionByAccessAndEntity(policy.getId(), - accessType.toString(), entityType)); + for (AccessType type : AccessType.values()) { + if (type.ordinal() >= accessType.ordinal()) { + permissions.addAll(_iamSrv.listPolicyPermissionByAccessAndEntity(policy.getId(), + type.toString(), entityType)); + } + } } } } else { if (accessType != null) { - permissions.addAll(_iamSrv.listPolicyPermissionByAccessAndEntity(policy.getId(), - accessType.toString(), entityType)); + for (AccessType type : AccessType.values()) { + if (type.ordinal() >= accessType.ordinal()) { + permissions.addAll(_iamSrv.listPolicyPermissionByAccessAndEntity(policy.getId(), + type.toString(), entityType)); + } + } } } for (IAMPolicyPermission permission : permissions) { @@ -145,6 +154,48 @@ public class RoleBasedEntityAccessChecker extends DomainChecker implements Secur return false; } + @Override + public boolean checkAccess(Account caller, AccessType accessType, String action, ControlledEntity... entities) + throws PermissionDeniedException { + + // operate access on multiple entities? + if (accessType != null && accessType == AccessType.OperateEntry) { + // In this case caller MUST own n-1 entities. + + for (ControlledEntity entity : entities) { + checkAccess(caller, entity, accessType, action); + + 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 (otherEntitiesAccess) { + return true; + } + } + + throw new PermissionDeniedException(caller + + " does not have permission to perform this operation on these resources"); + + } else { + for (ControlledEntity entity : entities) { + if (!checkAccess(caller, entity, accessType, action)) { + return false; + } + } + return true; + } + } + private boolean checkPermissionScope(Account caller, String scope, Long scopeId, ControlledEntity entity) { if(scopeId != null && !scopeId.equals(new Long(IAMPolicyPermission.PERMISSION_SCOPE_ID_CURRENT_CALLER))){ @@ -181,15 +232,8 @@ public class RoleBasedEntityAccessChecker extends DomainChecker implements Secur private List<IAMPolicy> getEffectivePolicies(Account caller, ControlledEntity entity) { - // Get the static Policies of the Caller List<IAMPolicy> policies = _iamSrv.listIAMPolicies(caller.getId()); - // add any dynamic policies w.r.t the entity - if (caller.getId() == entity.getAccountId()) { - // The caller owns the entity - policies.add(_iamSrv.getResourceOwnerPolicy()); - } - List<IAMGroup> groups = _iamSrv.listIAMGroups(caller.getId()); for (IAMGroup group : groups) { // for each group find the grand parent groups.
