cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mc...@apache.org
Subject [1/4] git commit: updated refs/heads/4.4 to 4367d14
Date Wed, 02 Apr 2014 00:39:23 GMT
Repository: cloudstack
Updated Branches:
  refs/heads/4.4 a8a0e84b8 -> 4367d1406


Change AccountManagerImpl.checkAccess to invoke SecurityChecker
interface that takes multiple controlled entities.


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/4367d140
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/4367d140
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/4367d140

Branch: refs/heads/4.4
Commit: 4367d1406b1d956d2a8d867af83ca4707b30193e
Parents: c89eb73
Author: Min Chen <min.chen@citrix.com>
Authored: Tue Apr 1 17:31:29 2014 -0700
Committer: Min Chen <min.chen@citrix.com>
Committed: Tue Apr 1 17:31:56 2014 -0700

----------------------------------------------------------------------
 api/src/com/cloud/user/AccountService.java      |  14 +--
 .../apache/cloudstack/acl/SecurityChecker.java  |   6 +-
 .../command/user/nat/DisableStaticNatCmd.java   |   5 +-
 .../command/user/nat/EnableStaticNatCmd.java    |   3 +-
 .../command/user/volume/AttachVolumeCmd.java    |   4 +-
 .../contrail/management/MockAccountManager.java |  11 --
 server/src/com/cloud/acl/DomainChecker.java     |  12 ++
 server/src/com/cloud/api/ApiDispatcher.java     |  22 ----
 .../cloud/api/dispatch/ParamProcessWorker.java  |  16 ++-
 .../com/cloud/storage/VolumeApiServiceImpl.java |   1 +
 .../src/com/cloud/user/AccountManagerImpl.java  | 115 ++++---------------
 .../com/cloud/user/MockAccountManagerImpl.java  |   9 --
 .../iam/RoleBasedEntityAccessChecker.java       |  19 +--
 13 files changed, 71 insertions(+), 166 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4367d140/api/src/com/cloud/user/AccountService.java
----------------------------------------------------------------------
diff --git a/api/src/com/cloud/user/AccountService.java b/api/src/com/cloud/user/AccountService.java
index 4965270..4b5dc36 100755
--- a/api/src/com/cloud/user/AccountService.java
+++ b/api/src/com/cloud/user/AccountService.java
@@ -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);
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4367d140/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 4170871..79366bd 100644
--- a/api/src/org/apache/cloudstack/acl/SecurityChecker.java
+++ b/api/src/org/apache/cloudstack/acl/SecurityChecker.java
@@ -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,
     }
 
     /**

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4367d140/api/src/org/apache/cloudstack/api/command/user/nat/DisableStaticNatCmd.java
----------------------------------------------------------------------
diff --git a/api/src/org/apache/cloudstack/api/command/user/nat/DisableStaticNatCmd.java b/api/src/org/apache/cloudstack/api/command/user/nat/DisableStaticNatCmd.java
index 1df77ec..2a9311e 100644
--- a/api/src/org/apache/cloudstack/api/command/user/nat/DisableStaticNatCmd.java
+++ b/api/src/org/apache/cloudstack/api/command/user/nat/DisableStaticNatCmd.java
@@ -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");
         }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4367d140/api/src/org/apache/cloudstack/api/command/user/nat/EnableStaticNatCmd.java
----------------------------------------------------------------------
diff --git a/api/src/org/apache/cloudstack/api/command/user/nat/EnableStaticNatCmd.java b/api/src/org/apache/cloudstack/api/command/user/nat/EnableStaticNatCmd.java
index 94699ac..9d88876 100644
--- a/api/src/org/apache/cloudstack/api/command/user/nat/EnableStaticNatCmd.java
+++ b/api/src/org/apache/cloudstack/api/command/user/nat/EnableStaticNatCmd.java
@@ -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());

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4367d140/api/src/org/apache/cloudstack/api/command/user/volume/AttachVolumeCmd.java
----------------------------------------------------------------------
diff --git a/api/src/org/apache/cloudstack/api/command/user/volume/AttachVolumeCmd.java b/api/src/org/apache/cloudstack/api/command/user/volume/AttachVolumeCmd.java
index 53b6bb6..b3bb37e 100644
--- a/api/src/org/apache/cloudstack/api/command/user/volume/AttachVolumeCmd.java
+++ b/api/src/org/apache/cloudstack/api/command/user/volume/AttachVolumeCmd.java
@@ -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;
 

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4367d140/plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/MockAccountManager.java
----------------------------------------------------------------------
diff --git a/plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/MockAccountManager.java
b/plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/MockAccountManager.java
index 8fa6fed..1b4b96e 100644
--- a/plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/MockAccountManager.java
+++ b/plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/MockAccountManager.java
@@ -96,12 +96,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
         return null;
@@ -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) {

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4367d140/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 14976cc..da39f51 100755
--- a/server/src/com/cloud/acl/DomainChecker.java
+++ b/server/src/com/cloud/acl/DomainChecker.java
@@ -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;

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4367d140/server/src/com/cloud/api/ApiDispatcher.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/api/ApiDispatcher.java b/server/src/com/cloud/api/ApiDispatcher.java
index 95074e2..3026bee 100755
--- a/server/src/com/cloud/api/ApiDispatcher.java
+++ b/server/src/com/cloud/api/ApiDispatcher.java
@@ -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));

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4367d140/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 c86689f..3c0c66b 100644
--- a/server/src/com/cloud/api/dispatch/ParamProcessWorker.java
+++ b/server/src/com/cloud/api/dispatch/ParamProcessWorker.java
@@ -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,
                         (ControlledEntity[]) entitiesToOperate.toArray());
             }
 

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4367d140/server/src/com/cloud/storage/VolumeApiServiceImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/com/cloud/storage/VolumeApiServiceImpl.java
index d70d7b2..680cd2e 100644
--- a/server/src/com/cloud/storage/VolumeApiServiceImpl.java
+++ b/server/src/com/cloud/storage/VolumeApiServiceImpl.java
@@ -1127,6 +1127,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, volume, vm);
 
         if (!(Volume.State.Allocated.equals(volume.getState()) || Volume.State.Ready.equals(volume.getState())
|| Volume.State.Uploaded.equals(volume.getState()))) {

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4367d140/server/src/com/cloud/user/AccountManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/user/AccountManagerImpl.java b/server/src/com/cloud/user/AccountManagerImpl.java
index 6de70c6..bd7f6a6 100755
--- a/server/src/com/cloud/user/AccountManagerImpl.java
+++ b/server/src/com/cloud/user/AccountManagerImpl.java
@@ -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;
-                }
-            }
-        }
-
-        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;
-        }
-
-        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;
+    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());
                 }
-            }
-
-            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);
+                granted = true;
+                break;
             }
         }
 
-        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;
-                }
-            }
+        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);
         }
-
-        // check that resources belong to the same account
-
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4367d140/server/test/com/cloud/user/MockAccountManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/test/com/cloud/user/MockAccountManagerImpl.java b/server/test/com/cloud/user/MockAccountManagerImpl.java
index f76f345..5938b3c 100644
--- a/server/test/com/cloud/user/MockAccountManagerImpl.java
+++ b/server/test/com/cloud/user/MockAccountManagerImpl.java
@@ -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) {

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/4367d140/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 295a106..cc29ab5 100644
--- a/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityAccessChecker.java
+++ b/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityAccessChecker.java
@@ -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;


Mime
View raw message