From 586ee74000bbbfb8724eb951ee860929be1ac645 Mon Sep 17 00:00:00 2001 From: Min Chen Date: Fri, 14 Feb 2014 11:23:05 -0800 Subject: [PATCH] Clean up SecurityChecker.AccessType and modify code to use them consistently. --- .../cloudstack/acl/SecurityChecker.java | 3 --- .../cloud/acl/AffinityGroupAccessChecker.java | 2 +- server/src/com/cloud/acl/DomainChecker.java | 2 +- .../cloud/server/ManagementServerImpl.java | 2 +- .../cloud/template/TemplateManagerImpl.java | 12 +++++------ .../affinity/AffinityGroupServiceImpl.java | 6 +++--- .../network/lb/CertServiceImpl.java | 21 ++++++++++--------- .../GlobalLoadBalancingRulesServiceImpl.java | 12 +++++------ .../cloudstack/acl/api/AclApiServiceImpl.java | 2 +- .../cloudstack/acl/AclApiServiceTest.java | 4 ++-- 10 files changed, 32 insertions(+), 34 deletions(-) diff --git a/api/src/org/apache/cloudstack/acl/SecurityChecker.java b/api/src/org/apache/cloudstack/acl/SecurityChecker.java index 3fdcfedb5ca..2c98e82dbc5 100644 --- a/api/src/org/apache/cloudstack/acl/SecurityChecker.java +++ b/api/src/org/apache/cloudstack/acl/SecurityChecker.java @@ -31,11 +31,8 @@ import com.cloud.utils.component.Adapter; public interface SecurityChecker extends Adapter { public enum AccessType { - ListEntry, - ModifyEntry, ModifyProject, UseNetwork, - DeleteEntry, OperateEntry, UseEntry } diff --git a/server/src/com/cloud/acl/AffinityGroupAccessChecker.java b/server/src/com/cloud/acl/AffinityGroupAccessChecker.java index 1c8ea648ccb..7bcecf0e115 100644 --- a/server/src/com/cloud/acl/AffinityGroupAccessChecker.java +++ b/server/src/com/cloud/acl/AffinityGroupAccessChecker.java @@ -49,7 +49,7 @@ public class AffinityGroupAccessChecker extends DomainChecker { AffinityGroup group = (AffinityGroup)entity; if (_affinityGroupService.isAdminControlledGroup(group)) { - if (accessType != null && accessType == AccessType.ModifyEntry + if (accessType != null && accessType == AccessType.OperateEntry && !_accountMgr.isRootAdmin(caller.getId())) { throw new PermissionDeniedException(caller + " does not have permission to operate with resource " + entity); diff --git a/server/src/com/cloud/acl/DomainChecker.java b/server/src/com/cloud/acl/DomainChecker.java index 6c3274dc183..3df71a7da03 100755 --- a/server/src/com/cloud/acl/DomainChecker.java +++ b/server/src/com/cloud/acl/DomainChecker.java @@ -123,7 +123,7 @@ public class DomainChecker extends AdapterBase implements SecurityChecker { } } else { // Domain admin and regular user can delete/modify only templates created by them - if (accessType != null && accessType == AccessType.ModifyEntry) { + if (accessType != null && accessType == AccessType.OperateEntry) { if (!_accountService.isRootAdmin(caller.getId()) && owner.getId() != caller.getId()) { // For projects check if the caller account can access the project account if (owner.getType() != Account.ACCOUNT_TYPE_PROJECT || !(_projectMgr.canAccessProjectAccount(caller, owner.getId()))) { diff --git a/server/src/com/cloud/server/ManagementServerImpl.java b/server/src/com/cloud/server/ManagementServerImpl.java index bd84692941a..350dfc7fa12 100755 --- a/server/src/com/cloud/server/ManagementServerImpl.java +++ b/server/src/com/cloud/server/ManagementServerImpl.java @@ -1815,7 +1815,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe } // do a permission check - _accountMgr.checkAccess(account, AccessType.ModifyEntry, true, template); + _accountMgr.checkAccess(account, AccessType.OperateEntry, true, template); if(cmd.isRoutingType() != null){ if (!_accountService.isRootAdmin(account.getId())) { diff --git a/server/src/com/cloud/template/TemplateManagerImpl.java b/server/src/com/cloud/template/TemplateManagerImpl.java index b500ed597d9..6b721398dc6 100755 --- a/server/src/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/com/cloud/template/TemplateManagerImpl.java @@ -367,7 +367,7 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, throw new InvalidParameterValueException("Unable to find template id=" + templateId); } - _accountMgr.checkAccess(CallContext.current().getCallingAccount(), AccessType.ModifyEntry, true, vmTemplate); + _accountMgr.checkAccess(CallContext.current().getCallingAccount(), AccessType.OperateEntry, true, vmTemplate); prepareTemplateInAllStoragePools(vmTemplate, zoneId); return vmTemplate; @@ -413,7 +413,7 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, throw new InvalidParameterValueException("Unable to extract template id=" + templateId + " as it's not extractable"); } - _accountMgr.checkAccess(caller, AccessType.ModifyEntry, true, template); + _accountMgr.checkAccess(caller, AccessType.OperateEntry, true, template); List ssStores = _dataStoreMgr.getImageStoresByScope(new ZoneScope(zoneId)); @@ -689,7 +689,7 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, return template; } - _accountMgr.checkAccess(caller, AccessType.ModifyEntry, true, template); + _accountMgr.checkAccess(caller, AccessType.OperateEntry, true, template); boolean success = copy(userId, template, srcSecStore, dstZone); @@ -1028,7 +1028,7 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, throw new InvalidParameterValueException("unable to find template with id " + templateId); } - _accountMgr.checkAccess(caller, AccessType.ModifyEntry, true, template); + _accountMgr.checkAccess(caller, AccessType.OperateEntry, true, template); if (template.getFormat() == ImageFormat.ISO) { throw new InvalidParameterValueException("Please specify a valid template."); @@ -1051,7 +1051,7 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, throw new InvalidParameterValueException("unable to find iso with id " + templateId); } - _accountMgr.checkAccess(caller, AccessType.ModifyEntry, true, template); + _accountMgr.checkAccess(caller, AccessType.OperateEntry, true, template); if (template.getFormat() != ImageFormat.ISO) { throw new InvalidParameterValueException("Please specify a valid iso."); @@ -1752,7 +1752,7 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, } // do a permission check - _accountMgr.checkAccess(account, AccessType.ModifyEntry, true, template); + _accountMgr.checkAccess(account, AccessType.OperateEntry, true, template); if (cmd.isRoutingType() != null) { if (!_accountService.isRootAdmin(account.getId())) { throw new PermissionDeniedException("Parameter isrouting can only be specified by a Root Admin, permission denied"); diff --git a/server/src/org/apache/cloudstack/affinity/AffinityGroupServiceImpl.java b/server/src/org/apache/cloudstack/affinity/AffinityGroupServiceImpl.java index 9fc9f0f11b0..559d5ac0444 100644 --- a/server/src/org/apache/cloudstack/affinity/AffinityGroupServiceImpl.java +++ b/server/src/org/apache/cloudstack/affinity/AffinityGroupServiceImpl.java @@ -26,8 +26,8 @@ import javax.ejb.Local; import javax.inject.Inject; import javax.naming.ConfigurationException; -import org.apache.log4j.Logger; - +import org.apache.log4j.Logger; + import org.apache.cloudstack.acl.AclEntityType; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.ControlledEntity.ACLType; @@ -263,7 +263,7 @@ public class AffinityGroupServiceImpl extends ManagerBase implements AffinityGro affinityGroupId = group.getId(); } // check permissions - _accountMgr.checkAccess(caller, AccessType.ModifyEntry, true, group); + _accountMgr.checkAccess(caller, AccessType.OperateEntry, true, group); final Long affinityGroupIdFinal = affinityGroupId; Transaction.execute(new TransactionCallbackNoReturn() { diff --git a/server/src/org/apache/cloudstack/network/lb/CertServiceImpl.java b/server/src/org/apache/cloudstack/network/lb/CertServiceImpl.java index d3c5dc5c2de..ba71d631a94 100644 --- a/server/src/org/apache/cloudstack/network/lb/CertServiceImpl.java +++ b/server/src/org/apache/cloudstack/network/lb/CertServiceImpl.java @@ -53,17 +53,18 @@ import javax.crypto.NoSuchPaddingException; import javax.ejb.Local; import javax.inject.Inject; +import org.apache.commons.io.IOUtils; +import org.apache.log4j.Logger; +import org.bouncycastle.jce.provider.BouncyCastleProvider; +import org.bouncycastle.openssl.PEMReader; +import org.bouncycastle.openssl.PasswordFinder; + import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.api.command.user.loadbalancer.DeleteSslCertCmd; import org.apache.cloudstack.api.command.user.loadbalancer.ListSslCertsCmd; import org.apache.cloudstack.api.command.user.loadbalancer.UploadSslCertCmd; import org.apache.cloudstack.api.response.SslCertResponse; import org.apache.cloudstack.context.CallContext; -import org.apache.commons.io.IOUtils; -import org.apache.log4j.Logger; -import org.bouncycastle.jce.provider.BouncyCastleProvider; -import org.bouncycastle.openssl.PEMReader; -import org.bouncycastle.openssl.PasswordFinder; import com.cloud.event.ActionEvent; import com.cloud.event.EventTypes; @@ -146,7 +147,7 @@ public class CertServiceImpl implements CertService { if (certVO == null) { throw new InvalidParameterValueException("Invalid certificate id: " + certId); } - _accountMgr.checkAccess(caller, SecurityChecker.AccessType.ModifyEntry, true, certVO); + _accountMgr.checkAccess(caller, SecurityChecker.AccessType.OperateEntry, true, certVO); List lbCertRule = _lbCertDao.listByCertId(certId); @@ -190,7 +191,7 @@ public class CertServiceImpl implements CertService { throw new InvalidParameterValueException("Invalid certificate id: " + certId); } - _accountMgr.checkAccess(caller, SecurityChecker.AccessType.ListEntry, true, certVO); + _accountMgr.checkAccess(caller, SecurityChecker.AccessType.UseEntry, true, certVO); certLbMap = _lbCertDao.listByCertId(certId); @@ -205,7 +206,7 @@ public class CertServiceImpl implements CertService { throw new InvalidParameterValueException("found no loadbalancer wth id: " + lbRuleId); } - _accountMgr.checkAccess(caller, SecurityChecker.AccessType.ListEntry, true, lb); + _accountMgr.checkAccess(caller, SecurityChecker.AccessType.UseEntry, true, lb); // get the cert id LoadBalancerCertMapVO lbCertMapRule; @@ -228,7 +229,7 @@ public class CertServiceImpl implements CertService { List certVOList = _sslCertDao.listByAccountId(accountId); if (certVOList == null || certVOList.isEmpty()) return certResponseList; - _accountMgr.checkAccess(caller, SecurityChecker.AccessType.ListEntry, true, certVOList.get(0)); + _accountMgr.checkAccess(caller, SecurityChecker.AccessType.UseEntry, true, certVOList.get(0)); for (SslCertVO cert : certVOList) { certLbMap = _lbCertDao.listByCertId(cert.getId()); @@ -486,7 +487,7 @@ public class CertServiceImpl implements CertService { char[] password; KeyPassword(char[] word) { - this.password = word; + password = word; } @Override diff --git a/server/src/org/apache/cloudstack/region/gslb/GlobalLoadBalancingRulesServiceImpl.java b/server/src/org/apache/cloudstack/region/gslb/GlobalLoadBalancingRulesServiceImpl.java index e88d6f8b5c2..c84fea29d5e 100644 --- a/server/src/org/apache/cloudstack/region/gslb/GlobalLoadBalancingRulesServiceImpl.java +++ b/server/src/org/apache/cloudstack/region/gslb/GlobalLoadBalancingRulesServiceImpl.java @@ -98,7 +98,7 @@ public class GlobalLoadBalancingRulesServiceImpl implements GlobalLoadBalancingR protected GslbServiceProvider _gslbProvider = null; public void setGslbServiceProvider(GslbServiceProvider provider) { - this._gslbProvider = provider; + _gslbProvider = provider; } @Override @@ -183,7 +183,7 @@ public class GlobalLoadBalancingRulesServiceImpl implements GlobalLoadBalancingR throw new InvalidParameterValueException("Invalid global load balancer rule id: " + gslbRuleId); } - _accountMgr.checkAccess(caller, SecurityChecker.AccessType.ModifyEntry, true, gslbRule); + _accountMgr.checkAccess(caller, SecurityChecker.AccessType.OperateEntry, true, gslbRule); if (gslbRule.getState() == GlobalLoadBalancerRule.State.Revoke) { throw new InvalidParameterValueException("global load balancer rule id: " + gslbRule.getUuid() + " is in revoked state"); @@ -319,7 +319,7 @@ public class GlobalLoadBalancingRulesServiceImpl implements GlobalLoadBalancingR throw new InvalidParameterValueException("Invalid global load balancer rule id: " + gslbRuleId); } - _accountMgr.checkAccess(caller, SecurityChecker.AccessType.ModifyEntry, true, gslbRule); + _accountMgr.checkAccess(caller, SecurityChecker.AccessType.OperateEntry, true, gslbRule); if (gslbRule.getState() == GlobalLoadBalancerRule.State.Revoke) { throw new InvalidParameterValueException("global load balancer rule id: " + gslbRuleId + " is already in revoked state"); @@ -445,7 +445,7 @@ public class GlobalLoadBalancingRulesServiceImpl implements GlobalLoadBalancingR throw new InvalidParameterValueException("Invalid global load balancer rule id: " + gslbRuleId); } - _accountMgr.checkAccess(caller, SecurityChecker.AccessType.ModifyEntry, true, gslbRule); + _accountMgr.checkAccess(caller, SecurityChecker.AccessType.OperateEntry, true, gslbRule); if (gslbRule.getState() == com.cloud.region.ha.GlobalLoadBalancerRule.State.Staged) { if (s_logger.isDebugEnabled()) { @@ -523,7 +523,7 @@ public class GlobalLoadBalancingRulesServiceImpl implements GlobalLoadBalancingR CallContext ctx = CallContext.current(); Account caller = ctx.getCallingAccount(); - _accountMgr.checkAccess(caller, SecurityChecker.AccessType.ModifyEntry, true, gslbRule); + _accountMgr.checkAccess(caller, SecurityChecker.AccessType.OperateEntry, true, gslbRule); if (algorithm != null && !GlobalLoadBalancerRule.Algorithm.isValidAlgorithm(algorithm)) { throw new InvalidParameterValueException("Invalid Algorithm: " + algorithm); @@ -583,7 +583,7 @@ public class GlobalLoadBalancingRulesServiceImpl implements GlobalLoadBalancingR if (gslbRule == null) { throw new InvalidParameterValueException("Invalid gslb rule id specified"); } - _accountMgr.checkAccess(caller, org.apache.cloudstack.acl.SecurityChecker.AccessType.ListEntry, false, gslbRule); + _accountMgr.checkAccess(caller, org.apache.cloudstack.acl.SecurityChecker.AccessType.UseEntry, false, gslbRule); response.add(gslbRule); return response; diff --git a/services/iam/plugin/src/org/apache/cloudstack/acl/api/AclApiServiceImpl.java b/services/iam/plugin/src/org/apache/cloudstack/acl/api/AclApiServiceImpl.java index e2704d8ce1d..94db2423cc5 100644 --- a/services/iam/plugin/src/org/apache/cloudstack/acl/api/AclApiServiceImpl.java +++ b/services/iam/plugin/src/org/apache/cloudstack/acl/api/AclApiServiceImpl.java @@ -409,7 +409,7 @@ public class AclApiServiceImpl extends ManagerBase implements AclApiService, Man Class cmdClass = _apiServer.getCmdClass(action); AccessType accessType = null; if (BaseListCmd.class.isAssignableFrom(cmdClass)) { - accessType = AccessType.ListEntry; + accessType = AccessType.UseEntry; } return _iamSrv.addAclPermissionToAclPolicy(aclPolicyId, entityType, scope.toString(), scopeId, action, accessType.toString(), perm, recursive); diff --git a/services/iam/plugin/test/org/apache/cloudstack/acl/AclApiServiceTest.java b/services/iam/plugin/test/org/apache/cloudstack/acl/AclApiServiceTest.java index e4457f5f793..602e7082d59 100644 --- a/services/iam/plugin/test/org/apache/cloudstack/acl/AclApiServiceTest.java +++ b/services/iam/plugin/test/org/apache/cloudstack/acl/AclApiServiceTest.java @@ -274,11 +274,11 @@ public class AclApiServiceTest { when(_apiServer.getCmdClass("listVirtualMachines")).thenReturn(clz); when( _iamSrv.addAclPermissionToAclPolicy(policyId, AclEntityType.VirtualMachine.toString(), PermissionScope.RESOURCE.toString(), resId, "listVirtualMachines", - AccessType.ListEntry.toString(), Permission.Allow, false)).thenReturn(policy); + AccessType.UseEntry.toString(), Permission.Allow, false)).thenReturn(policy); _aclSrv.addAclPermissionToAclPolicy(policyId, AclEntityType.VirtualMachine.toString(), PermissionScope.RESOURCE, resId, "listVirtualMachines", Permission.Allow, false); Pair, Integer> policyList = new Pair, Integer>(policies, 1); List policyPerms = new ArrayList(); - AclPolicyPermission perm = new AclPolicyPermissionVO(policyId, "listVirtualMachines", AclEntityType.VirtualMachine.toString(), AccessType.ListEntry.toString(), + AclPolicyPermission perm = new AclPolicyPermissionVO(policyId, "listVirtualMachines", AclEntityType.VirtualMachine.toString(), AccessType.UseEntry.toString(), PermissionScope.RESOURCE.toString(), resId, Permission.Allow, false); policyPerms.add(perm);