From 057657868b43b38d10c2df77eaee591e04257447 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 4 Aug 2025 00:59:44 +0530 Subject: [PATCH 1/8] refactor: avoid redundant DB calls when checking for root admin account AccountService.isRootAdmin(Long) is currently invoked each time to check if an account or caller is a root admin, leading to repeated DB lookups. In most cases, the Account object is already available and should be used directly. For the caller, the root admin flag is now cached in CallContext to avoid repeated evaluations. Signed-off-by: Abhishek Kumar --- .../java/com/cloud/user/AccountService.java | 2 + .../affinitygroup/CreateAffinityGroupCmd.java | 3 +- .../affinitygroup/DeleteAffinityGroupCmd.java | 3 +- .../ChangeSharedFSDiskOfferingCmd.java | 4 +- .../ChangeSharedFSServiceOfferingCmd.java | 6 +- .../storage/sharedfs/CreateSharedFSCmd.java | 22 +- .../storage/sharedfs/RestartSharedFSCmd.java | 4 +- .../storage/sharedfs/StartSharedFSCmd.java | 4 +- .../storage/sharedfs/StopSharedFSCmd.java | 4 +- .../storage/sharedfs/UpdateSharedFSCmd.java | 9 +- .../cloudstack/context/CallContext.java | 13 +- .../context/CallContextListener.java | 6 +- .../cloudstack/context/CallContextTest.java | 6 +- .../acl/ProjectRoleBasedApiAccessChecker.java | 2 +- .../ratelimit/ApiRateLimitServiceImpl.java | 2 +- .../cluster/KubernetesClusterManagerImpl.java | 4 +- .../IntegrationTestConfiguration.java | 39 ++-- .../cloud/acl/AffinityGroupAccessChecker.java | 2 +- .../java/com/cloud/acl/DomainChecker.java | 18 +- .../java/com/cloud/api/ApiResponseHelper.java | 30 +-- .../main/java/com/cloud/api/ApiServer.java | 37 ++-- .../com/cloud/api/query/QueryManagerImpl.java | 26 +-- .../api/query/dao/AccountJoinDaoImpl.java | 3 +- .../query/dao/AffinityGroupJoinDaoImpl.java | 4 +- .../api/query/dao/DataCenterJoinDaoImpl.java | 2 +- .../query/dao/DiskOfferingJoinDaoImpl.java | 2 +- .../api/query/dao/DomainJoinDaoImpl.java | 2 +- .../query/dao/DomainRouterJoinDaoImpl.java | 4 +- .../cloud/api/query/dao/HostJoinDaoImpl.java | 2 +- .../api/query/dao/ImageStoreJoinDaoImpl.java | 4 +- .../query/dao/InstanceGroupJoinDaoImpl.java | 2 +- .../query/dao/ServiceOfferingJoinDaoImpl.java | 2 +- .../api/query/dao/SnapshotJoinDaoImpl.java | 2 +- .../api/query/dao/StoragePoolJoinDaoImpl.java | 4 +- .../api/query/dao/TemplateJoinDaoImpl.java | 6 +- .../api/query/dao/UserVmJoinDaoImpl.java | 4 +- .../api/query/dao/VolumeJoinDaoImpl.java | 4 +- .../ConfigurationManagerImpl.java | 8 +- .../deploy/DeploymentPlanningManagerImpl.java | 4 +- .../com/cloud/deploy/FirstFitPlanner.java | 11 - .../cloud/network/IpAddressManagerImpl.java | 46 ++-- .../com/cloud/network/NetworkModelImpl.java | 2 +- .../com/cloud/network/NetworkServiceImpl.java | 23 +- .../network/firewall/FirewallManagerImpl.java | 2 +- .../com/cloud/network/vpc/VpcManagerImpl.java | 8 +- .../cloud/projects/ProjectManagerImpl.java | 4 +- .../cloud/resource/ResourceManagerImpl.java | 4 +- .../ResourceLimitManagerImpl.java | 6 +- .../cloud/server/ManagementServerImpl.java | 6 +- .../cloud/servlet/ConsoleProxyServlet.java | 2 +- .../com/cloud/storage/StorageManagerImpl.java | 27 +-- .../cloud/storage/VolumeApiServiceImpl.java | 34 +-- .../storage/snapshot/SnapshotManagerImpl.java | 8 +- .../cloud/template/TemplateAdapterBase.java | 4 +- .../cloud/template/TemplateManagerImpl.java | 14 +- .../com/cloud/usage/UsageServiceImpl.java | 2 +- .../com/cloud/user/AccountManagerImpl.java | 59 +++-- .../com/cloud/user/DomainManagerImpl.java | 2 +- .../cloud/uuididentity/UUIDManagerImpl.java | 2 +- .../java/com/cloud/vm/UserVmManagerImpl.java | 22 +- .../acl/ProjectRoleManagerImpl.java | 27 +-- .../cloudstack/acl/RoleManagerImpl.java | 14 +- .../affinity/AffinityGroupServiceImpl.java | 8 +- .../cloudstack/backup/BackupManagerImpl.java | 5 +- .../ConsoleAccessManagerImpl.java | 5 +- .../storage/sharedfs/SharedFSServiceImpl.java | 3 +- .../java/com/cloud/acl/DomainCheckerTest.java | 3 +- .../com/cloud/api/ApiResponseHelperTest.java | 12 +- .../cloud/api/query/QueryManagerImplTest.java | 2 +- .../api/query/dao/UserVmJoinDaoImplTest.java | 4 +- .../DeploymentPlanningManagerImplTest.java | 2 +- .../cloud/network/NetworkServiceImplTest.java | 2 +- .../ResourceLimitManagerImplTest.java | 61 +++-- .../cloud/storage/StorageManagerImplTest.java | 74 ++++--- .../snapshot/SnapshotManagerImplTest.java | 2 +- .../storage/snapshot/SnapshotManagerTest.java | 12 +- .../cloud/user/AccountManagerImplTest.java | 144 ++++++------ .../cloud/user/MockAccountManagerImpl.java | 6 + .../com/cloud/vm/UserVmManagerImplTest.java | 209 ++++++++---------- .../java/com/cloud/vm/UserVmManagerTest.java | 22 +- .../cloudstack/acl/RoleManagerImplTest.java | 67 +++--- .../cloudstack/backup/BackupManagerTest.java | 124 +++++------ .../ConsoleAccessManagerImplTest.java | 49 ++-- .../sharedfs/SharedFSServiceImplTest.java | 4 +- 84 files changed, 699 insertions(+), 755 deletions(-) diff --git a/api/src/main/java/com/cloud/user/AccountService.java b/api/src/main/java/com/cloud/user/AccountService.java index c0ebcf09f59b..4a2b5108962a 100644 --- a/api/src/main/java/com/cloud/user/AccountService.java +++ b/api/src/main/java/com/cloud/user/AccountService.java @@ -85,6 +85,8 @@ User createUser(String userName, String password, String firstName, String lastN boolean isRootAdmin(Long accountId); + boolean isRootAdmin(Account account); + boolean isDomainAdmin(Long accountId); boolean isResourceDomainAdmin(Long accountId); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/affinitygroup/CreateAffinityGroupCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/affinitygroup/CreateAffinityGroupCmd.java index ee0a38ef35dc..aa9a98baab1e 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/affinitygroup/CreateAffinityGroupCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/affinitygroup/CreateAffinityGroupCmd.java @@ -106,7 +106,8 @@ public long getEntityOwnerId() { Account caller = CallContext.current().getCallingAccount(); //For domain wide affinity groups (if the affinity group processor type allows it) - if(projectId == null && domainId != null && accountName == null && _accountService.isRootAdmin(caller.getId())){ + if(projectId == null && domainId != null && accountName == null && + CallContext.current().isCallingAccountRootAdmin()){ return Account.ACCOUNT_ID_SYSTEM; } Account owner = _accountService.finalizeOwner(caller, accountName, domainId, projectId); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/affinitygroup/DeleteAffinityGroupCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/affinitygroup/DeleteAffinityGroupCmd.java index 2f24158cadbb..90e6ccd7c71f 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/affinitygroup/DeleteAffinityGroupCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/affinitygroup/DeleteAffinityGroupCmd.java @@ -95,7 +95,8 @@ public long getEntityOwnerId() { Account caller = CallContext.current().getCallingAccount(); //For domain wide affinity groups (if the affinity group processor type allows it) - if(projectId == null && domainId != null && accountName == null && _accountService.isRootAdmin(caller.getId())){ + if(projectId == null && domainId != null && accountName == null && + CallContext.current().isCallingAccountRootAdmin()){ return Account.ACCOUNT_ID_SYSTEM; } Account owner = _accountService.finalizeOwner(caller, accountName, domainId, projectId); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/ChangeSharedFSDiskOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/ChangeSharedFSDiskOfferingCmd.java index b078ce4aae95..06eb9edb7d79 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/ChangeSharedFSDiskOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/ChangeSharedFSDiskOfferingCmd.java @@ -35,7 +35,6 @@ import com.cloud.event.EventTypes; import com.cloud.exception.ResourceAllocationException; -import com.cloud.user.Account; @APICommand(name = "changeSharedFileSystemDiskOffering", responseObject= SharedFSResponse.class, @@ -130,8 +129,7 @@ public void execute() throws ResourceAllocationException { SharedFS sharedFS = sharedFSService.changeSharedFSDiskOffering(this); if (sharedFS != null) { ResponseObject.ResponseView respView = getResponseView(); - Account caller = CallContext.current().getCallingAccount(); - if (_accountService.isRootAdmin(caller.getId())) { + if (CallContext.current().isCallingAccountRootAdmin()) { respView = ResponseObject.ResponseView.Full; } SharedFSResponse response = _responseGenerator.createSharedFSResponse(respView, sharedFS); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/ChangeSharedFSServiceOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/ChangeSharedFSServiceOfferingCmd.java index 70fb543d64c3..12894c9ea758 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/ChangeSharedFSServiceOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/ChangeSharedFSServiceOfferingCmd.java @@ -27,8 +27,8 @@ import org.apache.cloudstack.api.ResponseObject; import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.command.user.UserCmd; -import org.apache.cloudstack.api.response.SharedFSResponse; import org.apache.cloudstack.api.response.ServiceOfferingResponse; +import org.apache.cloudstack.api.response.SharedFSResponse; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.storage.sharedfs.SharedFS; import org.apache.cloudstack.storage.sharedfs.SharedFSService; @@ -39,7 +39,6 @@ import com.cloud.exception.OperationTimedoutException; import com.cloud.exception.ResourceUnavailableException; import com.cloud.exception.VirtualMachineMigrationException; -import com.cloud.user.Account; import com.cloud.utils.exception.CloudRuntimeException; @APICommand(name = "changeSharedFileSystemServiceOffering", @@ -132,8 +131,7 @@ public void execute() { if (sharedFS != null) { ResponseObject.ResponseView respView = getResponseView(); - Account caller = CallContext.current().getCallingAccount(); - if (_accountService.isRootAdmin(caller.getId())) { + if (CallContext.current().isCallingAccountRootAdmin()) { respView = ResponseObject.ResponseView.Full; } SharedFSResponse response = _responseGenerator.createSharedFSResponse(respView, sharedFS); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/CreateSharedFSCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/CreateSharedFSCmd.java index ddaa31612a89..e3164aaad41b 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/CreateSharedFSCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/CreateSharedFSCmd.java @@ -18,15 +18,6 @@ import javax.inject.Inject; -import com.cloud.event.EventTypes; -import com.cloud.exception.ConcurrentOperationException; -import com.cloud.exception.InsufficientCapacityException; -import com.cloud.exception.OperationTimedoutException; -import com.cloud.exception.ResourceAllocationException; -import com.cloud.exception.ResourceUnavailableException; -import com.cloud.user.Account; -import com.cloud.utils.exception.CloudRuntimeException; - import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiCommandResourceType; @@ -40,16 +31,24 @@ import org.apache.cloudstack.api.command.user.UserCmd; import org.apache.cloudstack.api.response.DiskOfferingResponse; import org.apache.cloudstack.api.response.DomainResponse; -import org.apache.cloudstack.api.response.SharedFSResponse; import org.apache.cloudstack.api.response.NetworkResponse; import org.apache.cloudstack.api.response.ProjectResponse; import org.apache.cloudstack.api.response.ServiceOfferingResponse; +import org.apache.cloudstack.api.response.SharedFSResponse; import org.apache.cloudstack.api.response.ZoneResponse; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.storage.sharedfs.SharedFS; import org.apache.cloudstack.storage.sharedfs.SharedFSProvider; import org.apache.cloudstack.storage.sharedfs.SharedFSService; +import com.cloud.event.EventTypes; +import com.cloud.exception.ConcurrentOperationException; +import com.cloud.exception.InsufficientCapacityException; +import com.cloud.exception.OperationTimedoutException; +import com.cloud.exception.ResourceAllocationException; +import com.cloud.exception.ResourceUnavailableException; +import com.cloud.utils.exception.CloudRuntimeException; + @APICommand(name = "createSharedFileSystem", responseObject= SharedFSResponse.class, description = "Create a new Shared File System of specified size and disk offering, attached to the given network", @@ -289,8 +288,7 @@ public void execute() { if (sharedFS != null) { ResponseObject.ResponseView respView = getResponseView(); - Account caller = CallContext.current().getCallingAccount(); - if (_accountService.isRootAdmin(caller.getId())) { + if (CallContext.current().isCallingAccountRootAdmin()) { respView = ResponseObject.ResponseView.Full; } SharedFSResponse response = _responseGenerator.createSharedFSResponse(respView, sharedFS); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/RestartSharedFSCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/RestartSharedFSCmd.java index 576c472b6eb2..8e7bb045bbe0 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/RestartSharedFSCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/RestartSharedFSCmd.java @@ -39,7 +39,6 @@ import com.cloud.exception.OperationTimedoutException; import com.cloud.exception.ResourceAllocationException; import com.cloud.exception.ResourceUnavailableException; -import com.cloud.user.Account; import com.cloud.utils.exception.CloudRuntimeException; @APICommand(name = "restartSharedFileSystem", @@ -130,8 +129,7 @@ public void execute() { if (sharedFS != null) { ResponseObject.ResponseView respView = getResponseView(); - Account caller = CallContext.current().getCallingAccount(); - if (_accountService.isRootAdmin(caller.getId())) { + if (CallContext.current().isCallingAccountRootAdmin()) { respView = ResponseObject.ResponseView.Full; } SharedFSResponse response = _responseGenerator.createSharedFSResponse(respView, sharedFS); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/StartSharedFSCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/StartSharedFSCmd.java index bd384aceef73..48eeaf9c5735 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/StartSharedFSCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/StartSharedFSCmd.java @@ -38,7 +38,6 @@ import com.cloud.exception.OperationTimedoutException; import com.cloud.exception.ResourceAllocationException; import com.cloud.exception.ResourceUnavailableException; -import com.cloud.user.Account; import com.cloud.utils.exception.CloudRuntimeException; @APICommand(name = "startSharedFileSystem", @@ -120,8 +119,7 @@ public void execute() { if (sharedFS != null) { ResponseObject.ResponseView respView = getResponseView(); - Account caller = CallContext.current().getCallingAccount(); - if (_accountService.isRootAdmin(caller.getId())) { + if (CallContext.current().isCallingAccountRootAdmin()) { respView = ResponseObject.ResponseView.Full; } SharedFSResponse response = _responseGenerator.createSharedFSResponse(respView, sharedFS); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/StopSharedFSCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/StopSharedFSCmd.java index d6e0737144a5..99bf624743e2 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/StopSharedFSCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/StopSharedFSCmd.java @@ -33,7 +33,6 @@ import org.apache.cloudstack.storage.sharedfs.SharedFSService; import com.cloud.event.EventTypes; -import com.cloud.user.Account; @APICommand(name = "stopSharedFileSystem", responseObject= SharedFSResponse.class, @@ -100,8 +99,7 @@ public void execute() { SharedFS sharedFS = sharedFSService.stopSharedFS(this.getId(), this.isForced()); if (sharedFS != null) { ResponseObject.ResponseView respView = getResponseView(); - Account caller = CallContext.current().getCallingAccount(); - if (_accountService.isRootAdmin(caller.getId())) { + if (CallContext.current().isCallingAccountRootAdmin()) { respView = ResponseObject.ResponseView.Full; } SharedFSResponse response = _responseGenerator.createSharedFSResponse(respView, sharedFS); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/UpdateSharedFSCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/UpdateSharedFSCmd.java index daad6cc78c56..258b6047a989 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/UpdateSharedFSCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/storage/sharedfs/UpdateSharedFSCmd.java @@ -16,6 +16,8 @@ // under the License. package org.apache.cloudstack.api.command.user.storage.sharedfs; +import javax.inject.Inject; + import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; @@ -30,10 +32,6 @@ import org.apache.cloudstack.storage.sharedfs.SharedFS; import org.apache.cloudstack.storage.sharedfs.SharedFSService; -import javax.inject.Inject; - -import com.cloud.user.Account; - @APICommand(name = "updateSharedFileSystem", responseObject= SharedFSResponse.class, description = "Update a Shared FileSystem", @@ -98,8 +96,7 @@ public void execute() { SharedFS sharedFS = sharedFSService.updateSharedFS(this); if (sharedFS != null) { ResponseObject.ResponseView respView = getResponseView(); - Account caller = CallContext.current().getCallingAccount(); - if (_accountService.isRootAdmin(caller.getId())) { + if (CallContext.current().isCallingAccountRootAdmin()) { respView = ResponseObject.ResponseView.Full; } SharedFSResponse response = _responseGenerator.createSharedFSResponse(respView, sharedFS); diff --git a/api/src/main/java/org/apache/cloudstack/context/CallContext.java b/api/src/main/java/org/apache/cloudstack/context/CallContext.java index 69376e4f6d7d..2b34ce934005 100644 --- a/api/src/main/java/org/apache/cloudstack/context/CallContext.java +++ b/api/src/main/java/org/apache/cloudstack/context/CallContext.java @@ -29,6 +29,7 @@ import com.cloud.exception.CloudAuthenticationException; import com.cloud.projects.Project; import com.cloud.user.Account; +import com.cloud.user.AccountService; import com.cloud.user.User; import com.cloud.utils.UuidUtils; import com.cloud.utils.db.EntityManager; @@ -53,6 +54,7 @@ protected Stack initialValue() { private String contextId; private Account account; private long accountId; + private Boolean isAccountRootAdmin = null; private long startEventId = 0; private String eventDescription; private String eventDetails; @@ -67,9 +69,11 @@ protected Stack initialValue() { private String apiName; static EntityManager s_entityMgr; + static AccountService accountService; - public static void init(EntityManager entityMgr) { + public static void init(EntityManager entityMgr, AccountService accountService) { s_entityMgr = entityMgr; + CallContext.accountService = accountService; } protected CallContext() { @@ -134,6 +138,13 @@ public Account getCallingAccount() { return account; } + public boolean isCallingAccountRootAdmin() { + if (isAccountRootAdmin == null) { + accountService.isRootAdmin(getCallingAccount()); + } + return Boolean.TRUE.equals(isAccountRootAdmin); + } + public static CallContext current() { CallContext context = s_currentContext.get(); diff --git a/api/src/main/java/org/apache/cloudstack/context/CallContextListener.java b/api/src/main/java/org/apache/cloudstack/context/CallContextListener.java index ab9a8c30046b..c6e2ceef83b8 100644 --- a/api/src/main/java/org/apache/cloudstack/context/CallContextListener.java +++ b/api/src/main/java/org/apache/cloudstack/context/CallContextListener.java @@ -23,6 +23,7 @@ import org.apache.cloudstack.managed.context.ManagedContextListener; +import com.cloud.user.AccountService; import com.cloud.utils.db.EntityManager; public class CallContextListener implements ManagedContextListener { @@ -30,6 +31,9 @@ public class CallContextListener implements ManagedContextListener { @Inject EntityManager entityMgr; + @Inject + AccountService accountService; + @Override public Object onEnterContext(boolean reentry) { if (!reentry && CallContext.current() == null) { @@ -47,6 +51,6 @@ public void onLeaveContext(Object unused, boolean reentry) { @PostConstruct public void init() { - CallContext.init(entityMgr); + CallContext.init(entityMgr, accountService); } } diff --git a/api/src/test/java/org/apache/cloudstack/context/CallContextTest.java b/api/src/test/java/org/apache/cloudstack/context/CallContextTest.java index d3537d6f8317..6f76dcb54726 100644 --- a/api/src/test/java/org/apache/cloudstack/context/CallContextTest.java +++ b/api/src/test/java/org/apache/cloudstack/context/CallContextTest.java @@ -31,6 +31,7 @@ import org.mockito.junit.MockitoJUnitRunner; import com.cloud.user.Account; +import com.cloud.user.AccountService; import com.cloud.user.User; import com.cloud.utils.db.EntityManager; @@ -40,9 +41,12 @@ public class CallContextTest { @Mock EntityManager entityMgr; + @Mock + AccountService accountService; + @Before public void setUp() { - CallContext.init(entityMgr); + CallContext.init(entityMgr, accountService); CallContext.register(Mockito.mock(User.class), Mockito.mock(Account.class)); } diff --git a/plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java b/plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java index 2e7ae23d6f1b..72980f288186 100644 --- a/plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java +++ b/plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java @@ -120,7 +120,7 @@ public boolean checkAccess(User user, String apiCommandName) throws PermissionDe } Account userAccount = accountService.getAccount(user.getAccountId()); - if (accountService.isRootAdmin(userAccount.getId()) || accountService.isDomainAdmin(userAccount.getAccountId())) { + if (accountService.isRootAdmin(userAccount) || accountService.isDomainAdmin(userAccount.getAccountId())) { logger.info(String.format("Account [%s] is Root Admin or Domain Admin, all APIs are allowed.", userAccount.getAccountName())); return true; } diff --git a/plugins/api/rate-limit/src/main/java/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java b/plugins/api/rate-limit/src/main/java/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java index 917cd7bb2b46..9560fb3e731e 100644 --- a/plugins/api/rate-limit/src/main/java/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java +++ b/plugins/api/rate-limit/src/main/java/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java @@ -173,7 +173,7 @@ public boolean checkAccess(User user, String apiCommandName) throws PermissionDe @Override public boolean checkAccess(Account account, String commandName) { Long accountId = account.getAccountId(); - if (_accountService.isRootAdmin(accountId)) { + if (_accountService.isRootAdmin(account)) { logger.info(String.format("Account [%s] is Root Admin, in this case, API limit does not apply.", ReflectionToStringBuilderUtils.reflectOnlySelectedFields(account, "accountName", "uuid"))); return true; diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java index dc092608ff3a..1a06d4a52c44 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java @@ -830,7 +830,7 @@ public KubernetesClusterResponse createKubernetesClusterResponse(long kubernetes List vmList = kubernetesClusterVmMapDao.listByClusterId(kubernetesCluster.getId()); ResponseView respView = ResponseView.Restricted; Account caller = CallContext.current().getCallingAccount(); - if (accountService.isRootAdmin(caller.getId())) { + if (CallContext.current().isCallingAccountRootAdmin()) { respView = ResponseView.Full; } final String responseName = "virtualmachine"; @@ -867,7 +867,7 @@ public KubernetesClusterResponse createKubernetesClusterResponse(long kubernetes response.setEtcdIps(etcdIps); } response.setHasAnnotation(annotationDao.hasAnnotations(kubernetesCluster.getUuid(), - AnnotationService.EntityType.KUBERNETES_CLUSTER.name(), accountService.isRootAdmin(caller.getId()))); + AnnotationService.EntityType.KUBERNETES_CLUSTER.name(), CallContext.current().isCallingAccountRootAdmin())); response.setVirtualMachines(vmResponses); response.setAutoscalingEnabled(kubernetesCluster.getAutoscalingEnabled()); response.setMinSize(kubernetesCluster.getMinSize()); diff --git a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/IntegrationTestConfiguration.java b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/IntegrationTestConfiguration.java index 9903f0e74901..1871a90e38b4 100644 --- a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/IntegrationTestConfiguration.java +++ b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/IntegrationTestConfiguration.java @@ -21,22 +21,6 @@ import javax.inject.Inject; -import org.apache.cloudstack.framework.config.dao.ConfigurationGroupDaoImpl; -import org.apache.cloudstack.framework.config.dao.ConfigurationSubGroupDaoImpl; -import org.eclipse.jetty.security.IdentityService; -import org.mockito.ArgumentMatchers; -import org.mockito.Mockito; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.ComponentScan; -import org.springframework.context.annotation.ComponentScan.Filter; -import org.springframework.context.annotation.Configuration; -import org.springframework.context.annotation.FilterType; -import org.springframework.core.type.classreading.MetadataReader; -import org.springframework.core.type.classreading.MetadataReaderFactory; -import org.springframework.core.type.filter.TypeFilter; - import org.apache.cloudstack.acl.APIChecker; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.affinity.AffinityGroupService; @@ -62,6 +46,8 @@ import org.apache.cloudstack.framework.config.ConfigDepot; import org.apache.cloudstack.framework.config.ConfigDepotAdmin; import org.apache.cloudstack.framework.config.dao.ConfigurationDaoImpl; +import org.apache.cloudstack.framework.config.dao.ConfigurationGroupDaoImpl; +import org.apache.cloudstack.framework.config.dao.ConfigurationSubGroupDaoImpl; import org.apache.cloudstack.framework.jobs.AsyncJobDispatcher; import org.apache.cloudstack.framework.jobs.dao.AsyncJobDaoImpl; import org.apache.cloudstack.framework.jobs.dao.AsyncJobJoinMapDaoImpl; @@ -82,11 +68,24 @@ import org.apache.cloudstack.region.dao.RegionDaoImpl; import org.apache.cloudstack.spring.lifecycle.registry.ExtensionRegistry; import org.apache.cloudstack.storage.datastore.PrimaryDataStoreProviderManager; -import org.apache.cloudstack.storage.image.datastore.ImageStoreProviderManager; import org.apache.cloudstack.storage.datastore.db.ImageStoreDaoImpl; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDaoImpl; +import org.apache.cloudstack.storage.image.datastore.ImageStoreProviderManager; import org.apache.cloudstack.storage.image.db.TemplateDataStoreDaoImpl; import org.apache.cloudstack.usage.UsageService; +import org.eclipse.jetty.security.IdentityService; +import org.mockito.ArgumentMatchers; +import org.mockito.Mockito; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.ComponentScan; +import org.springframework.context.annotation.ComponentScan.Filter; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.FilterType; +import org.springframework.core.type.classreading.MetadataReader; +import org.springframework.core.type.classreading.MetadataReaderFactory; +import org.springframework.core.type.filter.TypeFilter; import com.cloud.acl.DomainChecker; import com.cloud.agent.AgentManager; @@ -268,6 +267,7 @@ import com.cloud.template.TemplateManager; import com.cloud.user.Account; import com.cloud.user.AccountDetailsDaoImpl; +import com.cloud.user.AccountService; import com.cloud.user.DomainManagerImpl; import com.cloud.user.ResourceLimitService; import com.cloud.user.User; @@ -296,8 +296,8 @@ import com.cloud.vm.dao.SecondaryStorageVmDaoImpl; import com.cloud.vm.dao.UserVmCloneSettingDaoImpl; import com.cloud.vm.dao.UserVmDaoImpl; -import com.cloud.vm.dao.VMInstanceDetailsDaoImpl; import com.cloud.vm.dao.VMInstanceDaoImpl; +import com.cloud.vm.dao.VMInstanceDetailsDaoImpl; import com.cloud.vm.snapshot.VMSnapshotManager; import com.cloud.vm.snapshot.dao.VMSnapshotDaoImpl; @@ -497,6 +497,7 @@ public EndPointSelector endPointSelector() { @Bean public EntityManager entityManager() { EntityManager mock = Mockito.mock(EntityManager.class); + AccountService accountServiceMock = Mockito.mock(AccountService.class); try { Mockito.when(mock.findById(ArgumentMatchers.same(Account.class), ArgumentMatchers.anyLong())).thenReturn(_accountDao.findById(Account.ACCOUNT_ID_SYSTEM)); Mockito.when(mock.findById(ArgumentMatchers.same(User.class), ArgumentMatchers.anyLong())).thenReturn(_userDao.findById(User.UID_SYSTEM)); @@ -524,7 +525,7 @@ public DataCenter answer(final InvocationOnMock invocation) throws Throwable { } catch (Exception e) { e.printStackTrace(); } - CallContext.init(mock); + CallContext.init(mock, accountServiceMock); return mock; } diff --git a/server/src/main/java/com/cloud/acl/AffinityGroupAccessChecker.java b/server/src/main/java/com/cloud/acl/AffinityGroupAccessChecker.java index a865ff19f7b2..bdb8117cc7a0 100644 --- a/server/src/main/java/com/cloud/acl/AffinityGroupAccessChecker.java +++ b/server/src/main/java/com/cloud/acl/AffinityGroupAccessChecker.java @@ -57,7 +57,7 @@ public boolean checkAccess(Account caller, ControlledEntity entity, AccessType a AffinityGroup group = (AffinityGroup)entity; if (_affinityGroupService.isAdminControlledGroup(group)) { - if (accessType == AccessType.OperateEntry && !_accountMgr.isRootAdmin(caller.getId())) { + if (accessType == AccessType.OperateEntry && !_accountMgr.isRootAdmin(caller)) { throw new PermissionDeniedException(caller + " does not have permission to operate with resource " + entity); } diff --git a/server/src/main/java/com/cloud/acl/DomainChecker.java b/server/src/main/java/com/cloud/acl/DomainChecker.java index 97832311b178..e6db9ab446b2 100644 --- a/server/src/main/java/com/cloud/acl/DomainChecker.java +++ b/server/src/main/java/com/cloud/acl/DomainChecker.java @@ -175,7 +175,7 @@ public boolean checkAccess(Account caller, ControlledEntity entity, AccessType a Account owner = _accountDao.findById(template.getAccountId()); // validate that the template is usable by the account if (!template.isPublicTemplate()) { - if (_accountService.isRootAdmin(caller.getId()) || (owner.getId() == caller.getId())) { + if (_accountService.isRootAdmin(caller) || (owner.getId() == caller.getId())) { return true; } //special handling for the project case @@ -192,7 +192,7 @@ public boolean checkAccess(Account caller, ControlledEntity entity, AccessType a } else { // Domain admin and regular user can delete/modify only templates created by them if (accessType != null && accessType == AccessType.OperateEntry) { - if (!_accountService.isRootAdmin(caller.getId()) && owner.getId() != caller.getId()) { + if (!_accountService.isRootAdmin(caller) && owner.getId() != caller.getId()) { // For projects check if the caller account can access the project account if (owner.getType() != Account.Type.PROJECT || !(_projectMgr.canAccessProjectAccount(caller, owner.getId()))) { throw new PermissionDeniedException("Domain Admin and regular users can modify only their own Public templates"); @@ -221,7 +221,7 @@ public boolean checkAccess(Account caller, ControlledEntity entity, AccessType a protected void validateCallerHasAccessToEntityOwner(Account caller, ControlledEntity entity, AccessType accessType) { PermissionDeniedException exception = new PermissionDeniedException("Caller does not have permission to operate with provided resource."); - if (_accountService.isRootAdmin(caller.getId())) { + if (_accountService.isRootAdmin(caller)) { return; } @@ -272,7 +272,7 @@ protected boolean checkOperationPermitted(Account caller, ControlledEntity entit ProjectAccount projectUser = _projectAccountDao.findByProjectIdUserId(project.getId(), user.getAccountId(), user.getId()); String apiCommandName = CallContext.current().getApiName(); - if (accountService.isRootAdmin(caller.getId()) || accountService.isDomainAdmin(caller.getAccountId())) { + if (accountService.isRootAdmin(caller) || accountService.isDomainAdmin(caller.getAccountId())) { return true; } @@ -330,7 +330,7 @@ public boolean checkAccess(Account account, DiskOffering dof, DataCenter zone) t hasAccess = true; } else { //admin has all permissions - if (_accountService.isRootAdmin(account.getId())) { + if (_accountService.isRootAdmin(account)) { hasAccess = true; } //if account is normal user or domain admin @@ -368,7 +368,7 @@ public boolean checkAccess(Account account, ServiceOffering so, DataCenter zone) hasAccess = true; } else { //admin has all permissions - if (_accountService.isRootAdmin(account.getId())) { + if (_accountService.isRootAdmin(account)) { hasAccess = true; } //if account is normal user or domain admin @@ -406,7 +406,7 @@ public boolean checkAccess(Account account, NetworkOffering nof, DataCenter zone hasAccess = true; } else { //admin has all permissions - if (_accountService.isRootAdmin(account.getId())) { + if (_accountService.isRootAdmin(account)) { hasAccess = true; } //if account is normal user or domain admin @@ -444,7 +444,7 @@ public boolean checkAccess(Account account, VpcOffering vof, DataCenter zone) th hasAccess = true; } else { //admin has all permissions - if (_accountService.isRootAdmin(account.getId())) { + if (_accountService.isRootAdmin(account)) { hasAccess = true; } //if account is normal user or domain admin @@ -480,7 +480,7 @@ public boolean checkAccess(Account account, DataCenter zone) throws PermissionDe return true; } else { //admin has all permissions - if (_accountService.isRootAdmin(account.getId())) { + if (_accountService.isRootAdmin(account)) { return true; } //if account is normal user diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java b/server/src/main/java/com/cloud/api/ApiResponseHelper.java index 7a11a83c1801..962901cf5799 100644 --- a/server/src/main/java/com/cloud/api/ApiResponseHelper.java +++ b/server/src/main/java/com/cloud/api/ApiResponseHelper.java @@ -761,7 +761,7 @@ public SnapshotResponse createSnapshotResponse(Snapshot snapshot) { } snapshotResponse.setTags(new HashSet<>(tagResponses)); snapshotResponse.setHasAnnotation(annotationDao.hasAnnotations(snapshot.getUuid(), AnnotationService.EntityType.SNAPSHOT.name(), - _accountMgr.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); snapshotResponse.setObjectName("snapshot"); return snapshotResponse; @@ -846,7 +846,7 @@ public VMSnapshotResponse createVMSnapshotResponse(VMSnapshot vmSnapshot) { } vmSnapshotResponse.setTags(new HashSet<>(tagResponses)); vmSnapshotResponse.setHasAnnotation(annotationDao.hasAnnotations(vmSnapshot.getUuid(), AnnotationService.EntityType.VM_SNAPSHOT.name(), - _accountMgr.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); vmSnapshotResponse.setCurrent(vmSnapshot.getCurrent()); vmSnapshotResponse.setType(vmSnapshot.getType().toString()); @@ -1204,7 +1204,7 @@ public IPAddressResponse createIPAddressResponse(ResponseView view, IpAddress ip } ipResponse.setTags(tagResponses); ipResponse.setHasAnnotation(annotationDao.hasAnnotations(ipAddr.getUuid(), AnnotationService.EntityType.PUBLIC_IP_ADDRESS.name(), - _accountMgr.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); ipResponse.setHasRules(firewallRulesDao.countRulesByIpId(ipAddr.getId()) > 0); ipResponse.setObjectName("ipaddress"); @@ -1438,7 +1438,7 @@ public PodResponse createPodResponse(Pod pod, Boolean showCapacities) { } podResponse.setHasAnnotation(annotationDao.hasAnnotations(pod.getUuid(), AnnotationService.EntityType.POD.name(), - _accountMgr.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); podResponse.setObjectName("pod"); return podResponse; } @@ -1631,7 +1631,7 @@ public ClusterResponse createClusterResponse(Cluster cluster, Boolean showCapaci clusterResponse.setCapacities(new ArrayList(capacityResponses)); } clusterResponse.setHasAnnotation(annotationDao.hasAnnotations(cluster.getUuid(), AnnotationService.EntityType.CLUSTER.name(), - _accountMgr.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); clusterResponse.setObjectName("cluster"); return clusterResponse; } @@ -1839,7 +1839,7 @@ public SystemVmResponse createSystemVmResponse(VirtualMachine vm) { } vmResponse.setHasAnnotation(annotationDao.hasAnnotations(vm.getUuid(), AnnotationService.EntityType.SYSTEM_VM.name(), - _accountMgr.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); List nicProfiles = ApiDBUtils.getNics(vm); for (NicProfile singleNicProfile : nicProfiles) { Network network = ApiDBUtils.findNetworkById(singleNicProfile.getNetworkId()); @@ -2499,7 +2499,7 @@ public NetworkOfferingResponse createNetworkOfferingResponse(NetworkOffering off response.setDetails(details); } response.setHasAnnotation(annotationDao.hasAnnotations(offering.getUuid(), AnnotationService.EntityType.NETWORK_OFFERING.name(), - _accountMgr.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); return response; } @@ -2790,7 +2790,7 @@ public NetworkResponse createNetworkResponse(ResponseView view, Network network) } response.setTags(tagResponses); response.setHasAnnotation(annotationDao.hasAnnotations(network.getUuid(), AnnotationService.EntityType.NETWORK.name(), - _accountMgr.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); if (network.getNetworkACLId() != null) { NetworkACL acl = ApiDBUtils.findByNetworkACLId(network.getNetworkACLId()); @@ -3574,7 +3574,7 @@ public VpcResponse createVpcResponse(ResponseView view, Vpc vpc) { } response.setTags(tagResponses); response.setHasAnnotation(annotationDao.hasAnnotations(vpc.getUuid(), AnnotationService.EntityType.VPC.name(), - _accountMgr.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); ipv6Service.updateIpv6RoutesForVpcResponse(vpc, response); response.setDns1(vpc.getIp4Dns1()); response.setDns2(vpc.getIp4Dns2()); @@ -3813,7 +3813,7 @@ public AutoScaleVmGroupResponse createAutoScaleVmGroupResponse(AutoScaleVmGroup } response.setHasAnnotation(annotationDao.hasAnnotations(vmGroup.getUuid(), AnnotationService.EntityType.AUTOSCALE_VM_GROUP.name(), - _accountMgr.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); populateOwner(response, vmGroup); return response; @@ -3900,7 +3900,7 @@ public Site2SiteCustomerGatewayResponse createSite2SiteCustomerGatewayResponse(S response.setSplitConnections(result.getSplitConnections()); response.setObjectName("vpncustomergateway"); response.setHasAnnotation(annotationDao.hasAnnotations(result.getUuid(), AnnotationService.EntityType.VPN_CUSTOMER_GATEWAY.name(), - _accountMgr.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); populateAccount(response, result.getAccountId()); populateDomain(response, result.getDomainId()); @@ -5051,7 +5051,7 @@ public SSHKeyPairResponse createSSHKeyPairResponse(SSHKeyPair sshkeyPair, boolea response.setDomainId(domain.getUuid()); response.setDomainName(domain.getName()); response.setHasAnnotation(annotationDao.hasAnnotations(sshkeyPair.getUuid(), AnnotationService.EntityType.SSH_KEYPAIR.name(), - _accountMgr.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); return response; } @@ -5060,7 +5060,7 @@ public UserDataResponse createUserDataResponse(UserData userData) { UserDataResponse response = new UserDataResponse(userData.getUuid(), userData.getName(), userData.getUserData(), userData.getParams()); populateOwner(response, userData); response.setHasAnnotation(annotationDao.hasAnnotations(userData.getUuid(), AnnotationService.EntityType.USER_DATA.name(), - _accountMgr.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); return response; } @@ -5596,8 +5596,8 @@ public void updateTemplateIsoResponsesForIcons(List responses, public GuiThemeResponse createGuiThemeResponse(GuiThemeJoin guiThemeJoin) { GuiThemeResponse guiThemeResponse = new GuiThemeResponse(); - Long callerId = CallContext.current().getCallingAccount().getAccountId(); - if (callerId != Account.ACCOUNT_ID_SYSTEM && _accountMgr.isRootAdmin(callerId)) { + long callerId = CallContext.current().getCallingAccount().getAccountId(); + if (callerId != Account.ACCOUNT_ID_SYSTEM && CallContext.current().isCallingAccountRootAdmin()) { guiThemeResponse.setId(guiThemeJoin.getUuid()); guiThemeResponse.setName(guiThemeJoin.getName()); guiThemeResponse.setDescription(guiThemeJoin.getDescription()); diff --git a/server/src/main/java/com/cloud/api/ApiServer.java b/server/src/main/java/com/cloud/api/ApiServer.java index e0737a6891de..318e181196db 100644 --- a/server/src/main/java/com/cloud/api/ApiServer.java +++ b/server/src/main/java/com/cloud/api/ApiServer.java @@ -16,6 +16,9 @@ // under the License. package com.cloud.api; +import static com.cloud.user.AccountManagerImpl.apiKeyAccess; +import static org.apache.cloudstack.user.UserPasswordResetManager.UserPasswordResetEnabled; + import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InterruptedIOException; @@ -57,15 +60,6 @@ import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; -import com.cloud.cluster.ManagementServerHostVO; -import com.cloud.cluster.dao.ManagementServerHostDao; -import com.cloud.user.Account; -import com.cloud.user.AccountManager; -import com.cloud.user.AccountManagerImpl; -import com.cloud.user.DomainManager; -import com.cloud.user.User; -import com.cloud.user.UserAccount; -import com.cloud.user.UserVO; import org.apache.cloudstack.acl.APIChecker; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; @@ -154,6 +148,8 @@ import com.cloud.api.dispatch.DispatchChainFactory; import com.cloud.api.dispatch.DispatchTask; import com.cloud.api.response.ApiResponseSerializer; +import com.cloud.cluster.ManagementServerHostVO; +import com.cloud.cluster.dao.ManagementServerHostDao; import com.cloud.domain.Domain; import com.cloud.domain.DomainVO; import com.cloud.domain.dao.DomainDao; @@ -172,11 +168,18 @@ import com.cloud.exception.UnavailableCommandException; import com.cloud.projects.dao.ProjectDao; import com.cloud.storage.VolumeApiService; +import com.cloud.user.Account; +import com.cloud.user.AccountManager; +import com.cloud.user.AccountManagerImpl; +import com.cloud.user.DomainManager; +import com.cloud.user.User; +import com.cloud.user.UserAccount; +import com.cloud.user.UserVO; import com.cloud.utils.ConstantTimeComparator; import com.cloud.utils.DateUtil; import com.cloud.utils.HttpUtils; -import com.cloud.utils.HttpUtils.ApiSessionKeySameSite; import com.cloud.utils.HttpUtils.ApiSessionKeyCheckOption; +import com.cloud.utils.HttpUtils.ApiSessionKeySameSite; import com.cloud.utils.Pair; import com.cloud.utils.ReflectUtil; import com.cloud.utils.StringUtils; @@ -192,9 +195,6 @@ import com.cloud.utils.net.NetUtils; import com.google.gson.reflect.TypeToken; -import static com.cloud.user.AccountManagerImpl.apiKeyAccess; -import static org.apache.cloudstack.user.UserPasswordResetManager.UserPasswordResetEnabled; - @Component public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiServerService, Configurable { private static final Logger ACCESSLOGGER = LogManager.getLogger("apiserver." + ApiServer.class.getName()); @@ -698,7 +698,7 @@ public String handleRequest(final Map params, final String responseType, final S } catch (final InsufficientCapacityException ex) { logger.info(ex.getMessage()); String errorMsg = ex.getMessage(); - if (!accountMgr.isRootAdmin(CallContext.current().getCallingAccount().getId())) { + if (!CallContext.current().isCallingAccountRootAdmin()) { // hide internal details to non-admin user for security reason errorMsg = BaseCmd.USER_ERROR_MESSAGE; } @@ -709,7 +709,7 @@ public String handleRequest(final Map params, final String responseType, final S } catch (final ResourceUnavailableException ex) { logger.info(ex.getMessage()); String errorMsg = ex.getMessage(); - if (!accountMgr.isRootAdmin(CallContext.current().getCallingAccount().getId())) { + if (!CallContext.current().isCallingAccountRootAdmin()) { // hide internal details to non-admin user for security reason errorMsg = BaseCmd.USER_ERROR_MESSAGE; } @@ -720,7 +720,7 @@ public String handleRequest(final Map params, final String responseType, final S } catch (final Exception ex) { logger.error("unhandled exception executing api command: " + ((command == null) ? "null" : command), ex); String errorMsg = ex.getMessage(); - if (!accountMgr.isRootAdmin(CallContext.current().getCallingAccount().getId())) { + if (!CallContext.current().isCallingAccountRootAdmin()) { // hide internal details to non-admin user for security reason errorMsg = BaseCmd.USER_ERROR_MESSAGE; } @@ -878,7 +878,7 @@ private void buildAsyncListResponse(final BaseListCmd command, final Account acc List jobs; // list all jobs for ROOT admin - if (accountMgr.isRootAdmin(account.getId())) { + if (accountMgr.isRootAdmin(account)) { jobs = asyncMgr.findInstancePendingAsyncJobs(command.getApiResourceType().toString(), null); } else { jobs = asyncMgr.findInstancePendingAsyncJobs(command.getApiResourceType().toString(), account.getId()); @@ -1413,8 +1413,7 @@ else if (cmdList.size() == 1) else { // determine the cmd class based on calling context ResponseView view = ResponseView.Restricted; - if (CallContext.current() != null - && accountMgr.isRootAdmin(CallContext.current().getCallingAccount().getId())) { + if (CallContext.current().isCallingAccountRootAdmin()) { view = ResponseView.Full; } for (Class cmdClass : cmdList) { diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index 1b60bdcc9e1a..5e36578cf902 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -855,7 +855,7 @@ private Pair, Integer> searchForEventsInternal(ListEventsCmd c private Pair, Integer> searchForEventIdsAndCount(ListEventsCmd cmd) { Account caller = CallContext.current().getCallingAccount(); - boolean isRootAdmin = accountMgr.isRootAdmin(caller.getId()); + boolean isRootAdmin = accountMgr.isRootAdmin(caller); List permittedAccounts = new ArrayList<>(); Long id = cmd.getId(); @@ -949,7 +949,7 @@ private Pair, Integer> searchForEventIdsAndCount(ListEventsCmd cmd) { accountMgr.buildACLSearchCriteria(sc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); // For end users display only enabled events - if (!accountMgr.isRootAdmin(caller.getId())) { + if (!accountMgr.isRootAdmin(caller)) { sc.setParameters("displayEvent", true); } @@ -1185,7 +1185,7 @@ public ListResponse searchForUserVMs(ListVMsCmd cmd) { ResponseView respView = ResponseView.Restricted; Account caller = CallContext.current().getCallingAccount(); - if (accountMgr.isRootAdmin(caller.getId())) { + if (accountMgr.isRootAdmin(caller)) { respView = ResponseView.Full; } List vmResponses = ViewResponseHelper.createUserVmResponse(respView, "virtualmachine", cmd.getDetails(), cmd.getAccumulate(), cmd.getShowUserData(), @@ -1314,7 +1314,7 @@ private Pair, Integer> searchForUserVMIdsAndCount(ListVMsCmd cmd) { isAdmin = true; } - if (accountMgr.isRootAdmin(caller.getId())) { + if (accountMgr.isRootAdmin(caller)) { isRootAdmin = true; podId = (Long) getObjectPossibleMethodValue(cmd, "getPodId"); clusterId = (Long) getObjectPossibleMethodValue(cmd, "getClusterId"); @@ -2563,8 +2563,7 @@ public ListResponse searchForVolumes(ListVolumesCmd cmd) { } ResponseView respView = cmd.getResponseView(); - Account account = CallContext.current().getCallingAccount(); - if (accountMgr.isRootAdmin(account.getAccountId())) { + if (CallContext.current().isCallingAccountRootAdmin()) { respView = ResponseView.Full; } @@ -2629,7 +2628,8 @@ private Pair, Integer> searchForVolumeIdsAndCount(ListVolumesCmd cmd) Long diskOfferingId = cmd.getDiskOfferingId(); Boolean display = cmd.getDisplay(); String state = cmd.getState(); - boolean shouldListSystemVms = shouldListSystemVms(cmd, caller.getId()); + boolean shouldListSystemVms = Boolean.TRUE.equals(cmd.getListSystemVms()) && + CallContext.registerPlaceHolderContext().isCallingAccountRootAdmin(); Long zoneId = cmd.getZoneId(); Long podId = cmd.getPodId(); @@ -2820,10 +2820,6 @@ private Pair, Integer> searchForVolumeIdsAndCount(ListVolumesCmd cmd) return new Pair<>(vmIds, count); } - private boolean shouldListSystemVms(ListVolumesCmd cmd, Long callerId) { - return Boolean.TRUE.equals(cmd.getListSystemVms()) && accountMgr.isRootAdmin(callerId); - } - @Override public ListResponse searchForDomains(ListDomainsCmd cmd) { Pair, Integer> result = searchForDomainsInternal(cmd); @@ -3692,11 +3688,11 @@ private Ternary, Integer, String[]> searchForDiskOfferingsIdsAndCount // root Account account = CallContext.current().getCallingAccount(); + boolean isRootAdmin = CallContext.current().isCallingAccountRootAdmin(); Object name = cmd.getDiskOfferingName(); Object id = cmd.getId(); Object keyword = cmd.getKeyword(); Long domainId = cmd.getDomainId(); - boolean isRootAdmin = accountMgr.isRootAdmin(account.getAccountId()); Long projectId = cmd.getProjectId(); String accountName = cmd.getAccountName(); boolean isRecursive = cmd.isRecursive(); @@ -3723,7 +3719,7 @@ private Ternary, Integer, String[]> searchForDiskOfferingsIdsAndCount // if a domainId is provided, we just return the disk offering // associated with this domain if (domainId != null && accountName == null) { - if (accountMgr.isRootAdmin(account.getId()) || isPermissible(account.getDomainId(), domainId)) { + if (accountMgr.isRootAdmin(account) || isPermissible(account.getDomainId(), domainId)) { // check if the user's domain == do's domain || user's domain is // a child of so's domain for non-root users SearchBuilder domainDetailsSearch = _diskOfferingDetailsDao.createSearchBuilder(); @@ -4012,14 +4008,14 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic final Account owner = accountMgr.finalizeOwner(caller, accountName, domainId, projectId); - if (!accountMgr.isRootAdmin(caller.getId()) && isSystem) { + if (!accountMgr.isRootAdmin(caller) && isSystem) { throw new InvalidParameterValueException("Only ROOT admins can access system offerings."); } // Keeping this logic consistent with domain specific zones // if a domainId is provided, we just return the so associated with this // domain - if (domainId != null && !accountMgr.isRootAdmin(caller.getId())) { + if (domainId != null && !accountMgr.isRootAdmin(caller)) { // check if the user's domain == so's domain || user's domain is a // child of so's domain if (!isPermissible(owner.getDomainId(), domainId)) { diff --git a/server/src/main/java/com/cloud/api/query/dao/AccountJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/AccountJoinDaoImpl.java index 9a301d440a99..b611d3162e31 100644 --- a/server/src/main/java/com/cloud/api/query/dao/AccountJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/AccountJoinDaoImpl.java @@ -22,6 +22,7 @@ import javax.inject.Inject; +import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.springframework.stereotype.Component; @@ -91,7 +92,7 @@ public AccountResponse newAccountResponse(ResponseView view, EnumSet dedicatedResources = dedicatedResourceDao.listByAffinityGroupId(vag.getId()); this.populateDedicatedResourcesField(dedicatedResources, agResponse); } diff --git a/server/src/main/java/com/cloud/api/query/dao/DataCenterJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/DataCenterJoinDaoImpl.java index f2d061c7bb0a..d1f63be12459 100644 --- a/server/src/main/java/com/cloud/api/query/dao/DataCenterJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/DataCenterJoinDaoImpl.java @@ -169,7 +169,7 @@ public ZoneResponse newDataCenterResponse(ResponseView view, DataCenterJoinVO da zoneResponse.setResourceDetails(ApiDBUtils.getResourceDetails(dataCenter.getId(), ResourceObjectType.Zone)); zoneResponse.setHasAnnotation(annotationDao.hasAnnotations(dataCenter.getUuid(), AnnotationService.EntityType.ZONE.name(), - _accountMgr.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); zoneResponse.setAllowUserSpecifyVRMtu(NetworkService.AllowUsersToSpecifyVRMtu.valueIn(dataCenter.getId())); zoneResponse.setRouterPrivateInterfaceMaxMtu(NetworkService.VRPrivateInterfaceMtu.valueIn(dataCenter.getId())); zoneResponse.setRouterPublicInterfaceMaxMtu(NetworkService.VRPublicInterfaceMtu.valueIn(dataCenter.getId())); diff --git a/server/src/main/java/com/cloud/api/query/dao/DiskOfferingJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/DiskOfferingJoinDaoImpl.java index e53f94fa0c3e..4d25c2a69dfb 100644 --- a/server/src/main/java/com/cloud/api/query/dao/DiskOfferingJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/DiskOfferingJoinDaoImpl.java @@ -117,7 +117,7 @@ public DiskOfferingResponse newDiskOfferingResponse(DiskOfferingJoinVO offering) diskOfferingResponse.setEncrypt(offering.getEncrypt()); diskOfferingResponse.setHasAnnotation(annotationDao.hasAnnotations(offering.getUuid(), AnnotationService.EntityType.DISK_OFFERING.name(), - accountManager.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); diskOfferingResponse.setTags(offering.getTags()); diskOfferingResponse.setCustomized(offering.isCustomized()); diff --git a/server/src/main/java/com/cloud/api/query/dao/DomainJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/DomainJoinDaoImpl.java index 4a0929744cf8..3a1b13cfb63f 100644 --- a/server/src/main/java/com/cloud/api/query/dao/DomainJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/DomainJoinDaoImpl.java @@ -94,7 +94,7 @@ public DomainResponse newDomainResponse(ResponseView view, EnumSet detail hostResponse.setJobStatus(host.getJobStatus()); } hostResponse.setHasAnnotation(annotationDao.hasAnnotations(host.getUuid(), AnnotationService.EntityType.HOST.name(), - accountManager.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); hostResponse.setAnnotation(host.getAnnotation()); hostResponse.setLastAnnotated(host.getLastAnnotated ()); hostResponse.setUsername(host.getUsername()); diff --git a/server/src/main/java/com/cloud/api/query/dao/ImageStoreJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/ImageStoreJoinDaoImpl.java index 9a0c271fdb48..c2258f7ac2f9 100644 --- a/server/src/main/java/com/cloud/api/query/dao/ImageStoreJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/ImageStoreJoinDaoImpl.java @@ -90,7 +90,7 @@ public ImageStoreResponse newImageStoreResponse(ImageStoreJoinVO ids) { osResponse.setDiskSizeUsed(secStorageStats.getByteUsed()); } osResponse.setHasAnnotation(annotationDao.hasAnnotations(ids.getUuid(), AnnotationService.EntityType.SECONDARY_STORAGE.name(), - accountManager.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); osResponse.setObjectName("imagestore"); return osResponse; @@ -100,7 +100,7 @@ public ImageStoreResponse newImageStoreResponse(ImageStoreJoinVO ids) { public ImageStoreResponse setImageStoreResponse(ImageStoreResponse response, ImageStoreJoinVO ids) { if (response.hasAnnotation() == null) { response.setHasAnnotation(annotationDao.hasAnnotations(ids.getUuid(), AnnotationService.EntityType.SECONDARY_STORAGE.name(), - accountManager.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); } return response; } diff --git a/server/src/main/java/com/cloud/api/query/dao/InstanceGroupJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/InstanceGroupJoinDaoImpl.java index 4605c20287bb..52eae074dd5c 100644 --- a/server/src/main/java/com/cloud/api/query/dao/InstanceGroupJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/InstanceGroupJoinDaoImpl.java @@ -62,7 +62,7 @@ public InstanceGroupResponse newInstanceGroupResponse(InstanceGroupJoinVO group) groupResponse.setName(group.getName()); groupResponse.setCreated(group.getCreated()); groupResponse.setHasAnnotation(annotationDao.hasAnnotations(group.getUuid(), AnnotationService.EntityType.INSTANCE_GROUP.name(), - accountManager.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); ApiResponseHelper.populateOwner(groupResponse, group); diff --git a/server/src/main/java/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java index 579425a68c13..818443168cdb 100644 --- a/server/src/main/java/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java @@ -203,7 +203,7 @@ public ServiceOfferingResponse newServiceOfferingResponse(ServiceOfferingJoinVO } offeringResponse.setHasAnnotation(annotationDao.hasAnnotations(offering.getUuid(), AnnotationService.EntityType.SERVICE_OFFERING.name(), - accountManager.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); return offeringResponse; } diff --git a/server/src/main/java/com/cloud/api/query/dao/SnapshotJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/SnapshotJoinDaoImpl.java index bca60383501f..78ad142c0f5b 100644 --- a/server/src/main/java/com/cloud/api/query/dao/SnapshotJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/SnapshotJoinDaoImpl.java @@ -209,7 +209,7 @@ public SnapshotResponse setSnapshotResponse(SnapshotResponse snapshotResponse, S if (snapshotResponse.hasAnnotation() == null) { snapshotResponse.setHasAnnotation(annotationDao.hasAnnotations(snapshot.getUuid(), AnnotationService.EntityType.SNAPSHOT.name(), - accountService.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); } return snapshotResponse; } diff --git a/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDaoImpl.java index ce38727e42e5..558637277ad4 100644 --- a/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDaoImpl.java @@ -187,7 +187,7 @@ public StoragePoolResponse newStoragePoolResponse(StoragePoolJoinVO pool, boolea poolResponse.setJobStatus(pool.getJobStatus()); } poolResponse.setHasAnnotation(annotationDao.hasAnnotations(pool.getUuid(), AnnotationService.EntityType.PRIMARY_STORAGE.name(), - accountManager.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); poolResponse.setObjectName("storagepool"); return poolResponse; @@ -221,7 +221,7 @@ public StoragePoolResponse setStoragePoolResponse(StoragePoolResponse response, } if (response.hasAnnotation() == null) { response.setHasAnnotation(annotationDao.hasAnnotations(sp.getUuid(), AnnotationService.EntityType.PRIMARY_STORAGE.name(), - accountManager.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); } return response; } diff --git a/server/src/main/java/com/cloud/api/query/dao/TemplateJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/TemplateJoinDaoImpl.java index 2fc3056af3e9..433c3b056e5f 100644 --- a/server/src/main/java/com/cloud/api/query/dao/TemplateJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/TemplateJoinDaoImpl.java @@ -323,7 +323,7 @@ public TemplateResponse newTemplateResponse(EnumSet } templateResponse.setHasAnnotation(annotationDao.hasAnnotations(template.getUuid(), AnnotationService.EntityType.TEMPLATE.name(), - _accountService.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); templateResponse.setDirectDownload(template.isDirectDownload()); templateResponse.setDeployAsIs(template.isDeployAsIs()); @@ -465,7 +465,7 @@ public TemplateResponse setTemplateResponse(EnumSet if (templateResponse.hasAnnotation() == null) { templateResponse.setHasAnnotation(annotationDao.hasAnnotations(template.getUuid(), AnnotationService.EntityType.TEMPLATE.name(), - _accountService.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); } return templateResponse; @@ -607,7 +607,7 @@ public TemplateResponse newIsoResponse(TemplateJoinVO iso, ResponseView view) { } } isoResponse.setHasAnnotation(annotationDao.hasAnnotations(iso.getUuid(), AnnotationService.EntityType.ISO.name(), - _accountService.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); isoResponse.setDirectDownload(iso.isDirectDownload()); if (iso.getArch() != null) { diff --git a/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java index 9762fc5ed3eb..14a04895b181 100644 --- a/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java @@ -417,7 +417,7 @@ public UserVmResponse newUserVmResponse(ResponseView view, String objectName, Us } userVmResponse.setHasAnnotation(annotationDao.hasAnnotations(userVm.getUuid(), - AnnotationService.EntityType.VM.name(), _accountMgr.isRootAdmin(caller.getId()))); + AnnotationService.EntityType.VM.name(), _accountMgr.isRootAdmin(caller))); if (details.contains(VMDetails.all) || details.contains(VMDetails.affgrp)) { Long affinityGroupId = userVm.getAffinityGroupId(); @@ -659,7 +659,7 @@ public UserVmResponse setUserVmResponse(ResponseView view, UserVmResponse userVm if (userVmData.hasAnnotation() == null) { userVmData.setHasAnnotation(annotationDao.hasAnnotations(uvo.getUuid(), - AnnotationService.EntityType.VM.name(), _accountMgr.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + AnnotationService.EntityType.VM.name(), CallContext.current().isCallingAccountRootAdmin())); } Long affinityGroupId = uvo.getAffinityGroupId(); diff --git a/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java index 4f5d984c969a..7b0a24bd5f22 100644 --- a/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java @@ -261,7 +261,7 @@ public VolumeResponse newVolumeResponse(ResponseView view, VolumeJoinVO volume) } volResponse.setHasAnnotation(annotationDao.hasAnnotations(volume.getUuid(), AnnotationService.EntityType.VOLUME.name(), - _accountMgr.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); volResponse.setExtractable(isExtractable); volResponse.setDisplayVolume(volume.isDisplayVolume()); @@ -316,7 +316,7 @@ public VolumeResponse setVolumeResponse(ResponseView view, VolumeResponse volDat } if (volData.hasAnnotation() == null) { volData.setHasAnnotation(annotationDao.hasAnnotations(vol.getUuid(), AnnotationService.EntityType.VOLUME.name(), - _accountMgr.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + CallContext.current().isCallingAccountRootAdmin())); } return volData; } diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 64b2a66f4525..e9d5f92ec197 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -1024,7 +1024,7 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP validateIpAddressRelatedConfigValues(name, value); validateConflictingConfigValue(name, value); - if (CATEGORY_SYSTEM.equals(category) && !_accountMgr.isRootAdmin(caller.getId())) { + if (CATEGORY_SYSTEM.equals(category) && !_accountMgr.isRootAdmin(caller)) { logger.warn("Only Root Admin is allowed to edit the configuration {}", name); throw new CloudRuntimeException("Only Root Admin is allowed to edit this configuration."); } @@ -1911,7 +1911,7 @@ public Pod createPodIpRange(final CreateManagementNetworkIpRangeCmd cmd) { final Account account = CallContext.current().getCallingAccount(); - if(!_accountMgr.isRootAdmin(account.getId())) { + if(!_accountMgr.isRootAdmin(account)) { throw new PermissionDeniedException(String.format("Cannot perform this operation, Calling account is not root admin: %s", account)); } @@ -2554,7 +2554,7 @@ public Pod createPod(final long zoneId, final String name, final String startIp, } final Account account = CallContext.current().getCallingAccount(); if (Grouping.AllocationState.Disabled == zone.getAllocationState() - && !_accountMgr.isRootAdmin(account.getId())) { + && !_accountMgr.isRootAdmin(account)) { throw new PermissionDeniedException(String.format("Cannot perform this operation, Zone is currently disabled: %s", zone)); } @@ -4936,7 +4936,7 @@ public Vlan createVlanAndPublicIpRange(final CreateVlanIpRangeCmd cmd) throws In // Check if zone is enabled final Account caller = CallContext.current().getCallingAccount(); if (Grouping.AllocationState.Disabled == zone.getAllocationState() - && !_accountMgr.isRootAdmin(caller.getId())) { + && !_accountMgr.isRootAdmin(caller)) { throw new PermissionDeniedException(String.format("Cannot perform this operation, Zone is currently disabled: %s", zone)); } diff --git a/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java b/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java index daaebb42a340..3a769289a78d 100644 --- a/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java +++ b/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java @@ -823,7 +823,7 @@ public void checkForNonDedicatedResources(VirtualMachineProfile vmProfile, DataC // check if zone is dedicated. if yes check if vm owner has access to it. DedicatedResourceVO dedicatedZone = _dedicatedDao.findByZoneId(dc.getId()); - if (dedicatedZone != null && !_accountMgr.isRootAdmin(vmProfile.getOwner().getId())) { + if (dedicatedZone != null && !_accountMgr.isRootAdmin(vmProfile.getOwner())) { long accountDomainId = vmProfile.getOwner().getDomainId(); long accountId = vmProfile.getOwner().getAccountId(); logger.debug("Zone [{}] is dedicated. Checking if account [{}] in domain [{}] can use this zone to deploy VM [{}].", @@ -1966,7 +1966,7 @@ private boolean isEnabledForAllocation(long zoneId, Long podId, Long clusterId) private boolean isRootAdmin(VirtualMachineProfile vmProfile) { if (vmProfile != null) { if (vmProfile.getOwner() != null) { - return _accountMgr.isRootAdmin(vmProfile.getOwner().getId()); + return _accountMgr.isRootAdmin(vmProfile.getOwner()); } else { return false; } diff --git a/server/src/main/java/com/cloud/deploy/FirstFitPlanner.java b/server/src/main/java/com/cloud/deploy/FirstFitPlanner.java index 3aab852ba7fc..6c782ea289aa 100644 --- a/server/src/main/java/com/cloud/deploy/FirstFitPlanner.java +++ b/server/src/main/java/com/cloud/deploy/FirstFitPlanner.java @@ -635,17 +635,6 @@ private void removeClustersWithoutMatchingTag(List clusterListForVmAllocat } - private boolean isRootAdmin(VirtualMachineProfile vmProfile) { - if (vmProfile != null) { - if (vmProfile.getOwner() != null) { - return accountMgr.isRootAdmin(vmProfile.getOwner().getId()); - } else { - return false; - } - } - return false; - } - @Override public boolean canHandle(VirtualMachineProfile vm, DeploymentPlan plan, ExcludeList avoid) { // check what the ServiceOffering says. If null, check the global config diff --git a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java index b99a2f553c43..e9767bc5dacc 100644 --- a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java +++ b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java @@ -19,6 +19,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Calendar; +import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.HashSet; @@ -28,26 +29,10 @@ import java.util.Random; import java.util.Set; import java.util.UUID; -import java.util.Collections; import java.util.stream.Collectors; import javax.inject.Inject; -import com.cloud.dc.VlanDetailsVO; -import com.cloud.dc.dao.VlanDetailsDao; -import com.cloud.network.dao.NetrisProviderDao; -import com.cloud.network.dao.NsxProviderDao; -import com.cloud.network.dao.PublicIpQuarantineDao; -import com.cloud.network.dao.RemoteAccessVpnDao; -import com.cloud.network.dao.Site2SiteVpnGatewayDao; -import com.cloud.network.element.NetrisProviderVO; -import com.cloud.network.element.NsxProviderVO; -import com.cloud.network.vo.PublicIpQuarantineVO; -import com.cloud.network.vpc.Vpc; -import com.cloud.network.vpc.VpcOffering; -import com.cloud.network.vpc.VpcOfferingServiceMapVO; -import com.cloud.network.vpc.dao.VpcOfferingServiceMapDao; -import com.cloud.resourcelimit.CheckedReservation; import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.annotation.AnnotationService; @@ -68,6 +53,7 @@ import org.apache.cloudstack.region.Region; import org.apache.cloudstack.reservation.dao.ReservationDao; import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.lang3.ObjectUtils; import com.cloud.agent.AgentManager; import com.cloud.alert.AlertManager; @@ -83,6 +69,7 @@ import com.cloud.dc.Pod; import com.cloud.dc.PodVlanMapVO; import com.cloud.dc.Vlan.VlanType; +import com.cloud.dc.VlanDetailsVO; import com.cloud.dc.VlanVO; import com.cloud.dc.dao.AccountVlanMapDao; import com.cloud.dc.dao.DataCenterDao; @@ -92,6 +79,7 @@ import com.cloud.dc.dao.HostPodDao; import com.cloud.dc.dao.PodVlanMapDao; import com.cloud.dc.dao.VlanDao; +import com.cloud.dc.dao.VlanDetailsDao; import com.cloud.deploy.DeployDestination; import com.cloud.domain.Domain; import com.cloud.domain.dao.DomainDao; @@ -124,19 +112,26 @@ import com.cloud.network.dao.IPAddressDao; import com.cloud.network.dao.IPAddressVO; import com.cloud.network.dao.LoadBalancerDao; +import com.cloud.network.dao.NetrisProviderDao; import com.cloud.network.dao.NetworkAccountDao; import com.cloud.network.dao.NetworkDao; -import com.cloud.network.dao.NetworkDetailsDao; import com.cloud.network.dao.NetworkDetailVO; +import com.cloud.network.dao.NetworkDetailsDao; import com.cloud.network.dao.NetworkDomainDao; import com.cloud.network.dao.NetworkServiceMapDao; +import com.cloud.network.dao.NsxProviderDao; import com.cloud.network.dao.PhysicalNetworkDao; import com.cloud.network.dao.PhysicalNetworkServiceProviderDao; import com.cloud.network.dao.PhysicalNetworkTrafficTypeDao; +import com.cloud.network.dao.PublicIpQuarantineDao; +import com.cloud.network.dao.RemoteAccessVpnDao; +import com.cloud.network.dao.Site2SiteVpnGatewayDao; import com.cloud.network.dao.UserIpv6AddressDao; import com.cloud.network.element.IpDeployer; import com.cloud.network.element.IpDeployingRequester; +import com.cloud.network.element.NetrisProviderVO; import com.cloud.network.element.NetworkElement; +import com.cloud.network.element.NsxProviderVO; import com.cloud.network.element.StaticNatServiceProvider; import com.cloud.network.guru.NetworkGuru; import com.cloud.network.lb.LoadBalancingRulesManager; @@ -147,12 +142,17 @@ import com.cloud.network.rules.RulesManager; import com.cloud.network.rules.StaticNat; import com.cloud.network.rules.dao.PortForwardingRulesDao; +import com.cloud.network.vo.PublicIpQuarantineVO; import com.cloud.network.vpc.NetworkACLManager; +import com.cloud.network.vpc.Vpc; import com.cloud.network.vpc.VpcManager; +import com.cloud.network.vpc.VpcOffering; +import com.cloud.network.vpc.VpcOfferingServiceMapVO; import com.cloud.network.vpc.VpcVO; import com.cloud.network.vpc.dao.PrivateIpDao; import com.cloud.network.vpc.dao.VpcDao; import com.cloud.network.vpc.dao.VpcOfferingDao; +import com.cloud.network.vpc.dao.VpcOfferingServiceMapDao; import com.cloud.network.vpn.RemoteAccessVpnService; import com.cloud.offering.NetworkOffering; import com.cloud.offering.NetworkOffering.Availability; @@ -161,6 +161,7 @@ import com.cloud.offerings.dao.NetworkOfferingDetailsDao; import com.cloud.offerings.dao.NetworkOfferingServiceMapDao; import com.cloud.org.Grouping; +import com.cloud.resourcelimit.CheckedReservation; import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.user.ResourceLimitService; @@ -201,7 +202,6 @@ import com.cloud.vm.dao.NicSecondaryIpDao; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDao; -import org.apache.commons.lang3.ObjectUtils; public class IpAddressManagerImpl extends ManagerBase implements IpAddressManager, Configurable { @@ -1316,8 +1316,7 @@ private String generateErrorMessageForOperationOnDisabledZone(String operation, public AcquirePodIpCmdResponse allocatePodIp(String zoneId, String podId) throws ConcurrentOperationException, ResourceAllocationException { DataCenter zone = _entityMgr.findByUuid(DataCenter.class, zoneId); - Account caller = CallContext.current().getCallingAccount(); - if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller.getId())) { + if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !CallContext.current().isCallingAccountRootAdmin()) { ResourceAllocationException ex = new ResourceAllocationException( generateErrorMessageForOperationOnDisabledZone("allocate Pod IP addresses", zone), ResourceType.network); throw ex; @@ -1331,7 +1330,7 @@ public AcquirePodIpCmdResponse allocatePodIp(String zoneId, String podId) throws if (podvo == null) throw new ResourceAllocationException("No such pod exists", ResourceType.network); - vo = _privateIPAddressDao.takeIpAddress(zone.getId(), podvo.getId(), 0, caller.getId() + "", false); + vo = _privateIPAddressDao.takeIpAddress(zone.getId(), podvo.getId(), 0, CallContext.current().getCallingAccountId() + "", false); if(vo == null) throw new ResourceAllocationException("Unable to allocate IP from this Pod", ResourceType.network); if (vo.getIpAddress() == null) @@ -1366,8 +1365,7 @@ public void releasePodIp(Long id) throws CloudRuntimeException { } // Verify permission DataCenter zone = _entityMgr.findById(DataCenter.class, ipVO.getDataCenterId()); - Account caller = CallContext.current().getCallingAccount(); - if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller.getId())) { + if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !CallContext.current().isCallingAccountRootAdmin()) { throw new CloudRuntimeException(generateErrorMessageForOperationOnDisabledZone("release Pod IP", zone)); } try { @@ -1411,7 +1409,7 @@ public IpAddress allocateIp(final Account ipOwner, final boolean isSystem, Accou checkPublicIpOnExternalProviderZone(zone, ipaddress); - if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller.getId())) { + if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller)) { // zone is of type DataCenter. See DataCenterVO.java. PermissionDeniedException ex = new PermissionDeniedException(generateErrorMessageForOperationOnDisabledZone("allocate IP addresses", zone)); ex.addProxyObject(zone.getUuid(), "zoneId"); diff --git a/server/src/main/java/com/cloud/network/NetworkModelImpl.java b/server/src/main/java/com/cloud/network/NetworkModelImpl.java index 2ee835d844e3..f56bb310922d 100644 --- a/server/src/main/java/com/cloud/network/NetworkModelImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkModelImpl.java @@ -1699,7 +1699,7 @@ public void checkCapabilityForProvider(Set providers, Service service, @Override public final void checkNetworkPermissions(Account caller, Network network) { - if (_accountMgr.isRootAdmin(caller.getAccountId()) && Boolean.TRUE.equals(AdminIsAllowedToDeployAnywhere.value())) { + if (_accountMgr.isRootAdmin(caller) && Boolean.TRUE.equals(AdminIsAllowedToDeployAnywhere.value())) { if (logger.isDebugEnabled()) { logger.debug("root admin is permitted to do stuff on every network"); } diff --git a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java index 8c86ed55df8f..805482afdc03 100644 --- a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java @@ -1406,7 +1406,7 @@ void validateNetworkCidrSize(Account caller, Integer cidrSize, String cidr, Netw if (NetworkOffering.NetworkMode.ROUTED.equals(networkOffering.getNetworkMode()) && routedIpv4Manager.isVirtualRouterGateway(networkOffering)) { if (cidr != null) { - if (!networkOffering.isForVpc() && !_accountMgr.isRootAdmin(caller.getId())) { + if (!networkOffering.isForVpc() && !_accountMgr.isRootAdmin(caller)) { throw new InvalidParameterValueException("Only root admin can set the gateway/netmask of Isolated networks with ROUTED mode"); } return; @@ -1512,6 +1512,7 @@ public Network createGuestNetwork(CreateNetworkCmd cmd) throws InsufficientCapac String name = cmd.getNetworkName(); String displayText = cmd.getDisplayText(); Account caller = CallContext.current().getCallingAccount(); + boolean isCallerRootAdmin = CallContext.current().isCallingAccountRootAdmin(); Long physicalNetworkId = cmd.getPhysicalNetworkId(); Long domainId = cmd.getDomainId(); Boolean subdomainAccess = cmd.getSubdomainAccess(); @@ -1689,7 +1690,7 @@ public Network createGuestNetwork(CreateNetworkCmd cmd) throws InsufficientCapac } if (StringUtils.isNotBlank(isolatedPvlan)) { - if (!_accountMgr.isRootAdmin(caller.getId())) { + if (!isCallerRootAdmin) { throw new InvalidParameterValueException("Only ROOT admin is allowed to create Private VLAN network"); } if (zone.getNetworkType() != NetworkType.Advanced || ntwkOff.getGuestType() == GuestType.Isolated) { @@ -1710,7 +1711,7 @@ public Network createGuestNetwork(CreateNetworkCmd cmd) throws InsufficientCapac performBasicPrivateVlanChecks(vlanId, secondaryVlanId, privateVlanType); - if (!_accountMgr.isRootAdmin(caller.getId())) { + if (!isCallerRootAdmin) { validateNetworkOfferingForNonRootAdminUser(ntwkOff); } @@ -1720,7 +1721,7 @@ public Network createGuestNetwork(CreateNetworkCmd cmd) throws InsufficientCapac } // Don't allow to specify vlan if the caller is not ROOT admin - if (!_accountMgr.isRootAdmin(caller.getId()) && (ntwkOff.isSpecifyVlan() || vlanId != null || bypassVlanOverlapCheck)) { + if (!isCallerRootAdmin && (ntwkOff.isSpecifyVlan() || vlanId != null || bypassVlanOverlapCheck)) { throw new InvalidParameterValueException("Only ROOT admin is allowed to specify vlanId or bypass vlan overlap check"); } @@ -1737,7 +1738,7 @@ public Network createGuestNetwork(CreateNetworkCmd cmd) throws InsufficientCapac if (ipv4) { // For non-root admins check cidr limit - if it's allowed by global config value - if (!_accountMgr.isRootAdmin(caller.getId()) && cidr != null) { + if (!isCallerRootAdmin && cidr != null) { String[] cidrPair = cidr.split("\\/"); int cidrSize = Integer.parseInt(cidrPair[1]); @@ -1984,7 +1985,7 @@ private ACLType getAclType(Account caller, NetworkOffering ntwkOff, ACLType aclT if (ntwkOff.getGuestType() == GuestType.Isolated || ntwkOff.getGuestType() == GuestType.L2) { aclType = ACLType.Account; } else if (ntwkOff.getGuestType() == GuestType.Shared) { - if (_accountMgr.isRootAdmin(caller.getId())) { + if (_accountMgr.isRootAdmin(caller)) { aclType = ACLType.Domain; } else if (_accountMgr.isNormalUser(caller.getId())) { aclType = ACLType.Account; @@ -1996,7 +1997,7 @@ private ACLType getAclType(Account caller, NetworkOffering ntwkOff, ACLType aclT } private void validateZoneAvailability(Account caller, DataCenter zone) { - if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller.getId())) { + if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller)) { // See DataCenterVO.java PermissionDeniedException ex = new PermissionDeniedException("Cannot perform this operation since specified Zone is currently disabled"); ex.addProxyObject(zone.getUuid(), "zoneId"); @@ -2379,6 +2380,7 @@ public Pair, Integer> searchForNetworks(ListNetworksCmd String keyword = cmd.getKeyword(); Long zoneId = cmd.getZoneId(); Account caller = CallContext.current().getCallingAccount(); + boolean isCallerRootAdmin = CallContext.current().isCallingAccountRootAdmin(); Long domainId = cmd.getDomainId(); String accountName = cmd.getAccountName(); String guestIpType = cmd.getGuestIpType(); @@ -2413,7 +2415,7 @@ public Pair, Integer> searchForNetworks(ListNetworksCmd // 1) default is system to false if not specified // 2) reset parameter to false if it's specified by a non-ROOT user - if (isSystem == null || !_accountMgr.isRootAdmin(caller.getId())) { + if (isSystem == null || !isCallerRootAdmin) { isSystem = false; } @@ -2868,9 +2870,6 @@ private void addSharedNetworksByDomainPathToSearch(SearchCriteria add @Override @ActionEvent(eventType = EventTypes.EVENT_NETWORK_DELETE, eventDescription = "deleting network", async = true) public boolean deleteNetwork(long networkId, boolean forced) { - - Account caller = CallContext.current().getCallingAccount(); - // Verify network id NetworkVO network = getNetworkVO(networkId, "Unable to find a network with the specified ID."); @@ -2891,7 +2890,7 @@ public boolean deleteNetwork(long networkId, boolean forced) { Account owner = _accountMgr.getAccount(network.getAccountId()); - if (forced && !_accountMgr.isRootAdmin(caller.getId())) { + if (forced && !CallContext.current().isCallingAccountRootAdmin()) { throw new InvalidParameterValueException("Delete network with 'forced' option can only be called by root admins"); } diff --git a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java index 4aee5fef48a6..21a000e967a4 100644 --- a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java +++ b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java @@ -842,7 +842,7 @@ protected boolean revokeFirewallRule(long ruleId, boolean apply, Account caller, throw new InvalidParameterValueException("Unable to find " + ruleId + " having purpose " + Arrays.asList(Purpose.Firewall, Purpose.Ipv6Firewall)); } - if (rule.getType() == FirewallRuleType.System && !_accountMgr.isRootAdmin(caller.getId())) { + if (rule.getType() == FirewallRuleType.System && !_accountMgr.isRootAdmin(caller)) { throw new InvalidParameterValueException("Only root admin can delete the system wide firewall rule"); } diff --git a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java index e4219c858da6..ef34a6eb046f 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -1267,7 +1267,7 @@ public Vpc createVpc(final long zoneId, final long vpcOffId, final long vpcOwner throw new InvalidParameterValueException("Network domain must be specified for region level VPC"); } - if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller.getId())) { + if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller)) { // See DataCenterVO.java final PermissionDeniedException ex = new PermissionDeniedException("Cannot perform this operation since specified Zone is currently disabled"); ex.addProxyObject(zone.getUuid(), "zoneId"); @@ -1347,7 +1347,7 @@ private void validateVpcCidrSize(Account caller, long accountId, VpcOffering vpc } if (routedIpv4Manager.isValidGateway(vpcOffering)) { if (cidr != null) { - if (!_accountMgr.isRootAdmin(caller.getId())) { + if (!_accountMgr.isRootAdmin(caller)) { throw new InvalidParameterValueException("Only root admin can set the gateway/netmask of VPC with ROUTED mode"); } return; @@ -2644,7 +2644,7 @@ private void validateVpcPrivateGatewayAssociateNetworkId(NetworkOfferingVO ntwkO throw new InvalidParameterValueException("vlanId and associatedNetworkId are mutually exclusive"); } Account caller = CallContext.current().getCallingAccount(); - if (!_accountMgr.isRootAdmin(caller.getId()) && (ntwkOff.isSpecifyVlan() || broadcastUri != null || bypassVlanOverlapCheck)) { + if (!_accountMgr.isRootAdmin(caller) && (ntwkOff.isSpecifyVlan() || broadcastUri != null || bypassVlanOverlapCheck)) { throw new InvalidParameterValueException("Only ROOT admin is allowed to specify vlanId or bypass vlan overlap check"); } if (ntwkOff.isSpecifyVlan() && broadcastUri == null) { @@ -2767,7 +2767,7 @@ public boolean deleteVpcPrivateGateway(final long gatewayId) throws ConcurrentOp } final Account caller = CallContext.current().getCallingAccount(); - if (!_accountMgr.isRootAdmin(caller.getId())) { + if (!_accountMgr.isRootAdmin(caller)) { _accountMgr.checkAccess(caller, null, false, gatewayVO); final NetworkVO networkVO = _ntwkDao.findById(gatewayVO.getNetworkId()); if (networkVO != null) { diff --git a/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java b/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java index 7a743e3ce767..1416b9e343b7 100644 --- a/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java +++ b/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java @@ -611,7 +611,7 @@ public Project findByNameAndDomainId(String name, long domainId) { @Override public boolean canAccessProjectAccount(Account caller, long accountId) { //ROOT admin always can access the project - if (_accountMgr.isRootAdmin(caller.getId())) { + if (_accountMgr.isRootAdmin(caller)) { return true; } else if (_accountMgr.isDomainAdmin(caller.getId())) { Account owner = _accountMgr.getAccount(accountId); @@ -632,7 +632,7 @@ public boolean canAccessProjectAccount(Account caller, long accountId) { @Override public boolean canModifyProjectAccount(Account caller, long accountId) { //ROOT admin always can access the project - if (_accountMgr.isRootAdmin(caller.getId())) { + if (_accountMgr.isRootAdmin(caller)) { return true; } else if (_accountMgr.isDomainAdmin(caller.getId())) { Account owner = _accountMgr.getAccount(accountId); diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index 27e7f27ae0e2..0897604afe23 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -476,7 +476,7 @@ public List discoverCluster(final AddClusterCmd cmd) throws I } final Account account = CallContext.current().getCallingAccount(); - if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(account.getId())) { + if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(account)) { final PermissionDeniedException ex = new PermissionDeniedException("Cannot perform this operation, Zone with specified id is currently disabled"); ex.addProxyObject(zone.getUuid(), "dcId"); throw ex; @@ -741,7 +741,7 @@ private List discoverHostsFull(final Long dcId, final Long podId, Long c } final Account account = CallContext.current().getCallingAccount(); - if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(account.getId())) { + if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(account)) { final PermissionDeniedException ex = new PermissionDeniedException("Cannot perform this operation, Zone with specified id is currently disabled"); ex.addProxyObject(zone.getUuid(), "dcId"); throw ex; diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index 9a6c8a85f18e..43709e73fd0a 100644 --- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java +++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java @@ -390,7 +390,7 @@ public long findCorrectResourceLimitForAccount(Account account, ResourceType typ long max = Resource.RESOURCE_UNLIMITED; // if resource limit is not found, then we treat it as unlimited // No limits for Root Admin accounts - if (_accountMgr.isRootAdmin(account.getId())) { + if (_accountMgr.isRootAdmin(account)) { return max; } @@ -648,7 +648,7 @@ public void checkResourceLimitWithTag(final Account account, final ResourceType Project project = null; // Don't place any limits on system or root admin accounts - if (_accountMgr.isRootAdmin(account.getId())) { + if (_accountMgr.isRootAdmin(account)) { return; } @@ -948,7 +948,7 @@ public ResourceLimitVO updateResourceLimit(Long accountId, Long domainId, Intege } //only Unlimited value is accepted if account is Root Admin - if (_accountMgr.isRootAdmin(account.getId()) && max.shortValue() != Resource.RESOURCE_UNLIMITED) { + if (_accountMgr.isRootAdmin(account) && max.shortValue() != Resource.RESOURCE_UNLIMITED) { throw new InvalidParameterValueException("Only " + Resource.RESOURCE_UNLIMITED + " limit is supported for Root Admin accounts"); } diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index 3245acfbbf2c..d45c7cd67278 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -1450,8 +1450,7 @@ protected Pair> filterUefiHostsForMigration(List a } private void validateVmForHostMigration(VirtualMachine vm) { - final Account caller = getCaller(); - if (!_accountMgr.isRootAdmin(caller.getId())) { + if (!CallContext.current().isCallingAccountRootAdmin()) { if (logger.isDebugEnabled()) { logger.debug("Caller is not a root admin, permission denied to migrate the VM"); } @@ -1754,8 +1753,7 @@ public Pair, List> listStorag public Pair, List> listStoragePoolsForMigrationOfVolumeInternal(final Long volumeId, Long newDiskOfferingId, Long newSize, Long newMinIops, Long newMaxIops, boolean keepSourceStoragePool, boolean bypassStorageTypeCheck, boolean bypassAccountCheck, String keyword) { if (!bypassAccountCheck) { - final Account caller = getCaller(); - if (!_accountMgr.isRootAdmin(caller.getId())) { + if (!CallContext.current().isCallingAccountRootAdmin()) { if (logger.isDebugEnabled()) { logger.debug("Caller is not a root admin, permission denied to migrate the volume"); } diff --git a/server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java b/server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java index 2b786a8f1efe..a8286cc2a45b 100644 --- a/server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java +++ b/server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java @@ -424,7 +424,7 @@ private boolean checkSessionPermision(HttpServletRequest req, long vmId, Account } // root admin can access anything - if (_accountMgr.isRootAdmin(accountObj.getId())) + if (_accountMgr.isRootAdmin(accountObj)) return true; switch (vm.getType()) { diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index f6bef8b2e8cf..21b7664c54b4 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -56,10 +56,6 @@ import javax.inject.Inject; -import com.cloud.dc.HostPodVO; -import com.cloud.dc.dao.HostPodDao; -import com.cloud.resource.ResourceManager; -import com.cloud.storage.dao.StoragePoolAndAccessGroupMapDao; import org.apache.cloudstack.annotation.AnnotationService; import org.apache.cloudstack.annotation.dao.AnnotationDao; import org.apache.cloudstack.api.ApiConstants; @@ -189,9 +185,11 @@ import com.cloud.cpu.CPU; import com.cloud.dc.ClusterVO; import com.cloud.dc.DataCenterVO; +import com.cloud.dc.HostPodVO; import com.cloud.dc.VsphereStoragePolicyVO; import com.cloud.dc.dao.ClusterDao; import com.cloud.dc.dao.DataCenterDao; +import com.cloud.dc.dao.HostPodDao; import com.cloud.dc.dao.VsphereStoragePolicyDao; import com.cloud.event.ActionEvent; import com.cloud.event.EventTypes; @@ -218,6 +216,7 @@ import com.cloud.offering.ServiceOffering; import com.cloud.org.Grouping; import com.cloud.org.Grouping.AllocationState; +import com.cloud.resource.ResourceManager; import com.cloud.resource.ResourceState; import com.cloud.server.ConfigurationServer; import com.cloud.server.ManagementServer; @@ -229,6 +228,7 @@ import com.cloud.storage.dao.BucketDao; import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.SnapshotDao; +import com.cloud.storage.dao.StoragePoolAndAccessGroupMapDao; import com.cloud.storage.dao.StoragePoolHostDao; import com.cloud.storage.dao.StoragePoolTagsDao; import com.cloud.storage.dao.StoragePoolWorkDao; @@ -241,7 +241,6 @@ import com.cloud.template.TemplateManager; import com.cloud.template.VirtualMachineTemplate; import com.cloud.upgrade.SystemVmTemplateRegistration; -import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.user.ResourceLimitService; import com.cloud.user.dao.UserDao; @@ -938,11 +937,11 @@ protected void checkNFSMountOptionsForCreate(Map details, Hyperv checkNfsMountOptions(details.get(ApiConstants.NFS_MOUNT_OPTIONS)); } - protected void checkNFSMountOptionsForUpdate(Map details, StoragePoolVO pool, Long accountId) throws InvalidParameterValueException { + protected void checkNFSMountOptionsForUpdate(Map details, StoragePoolVO pool) throws InvalidParameterValueException { if (!details.containsKey(ApiConstants.NFS_MOUNT_OPTIONS)) { return; } - if (!_accountMgr.isRootAdmin(accountId)) { + if (!CallContext.current().isCallingAccountRootAdmin()) { throw new PermissionDeniedException("Only root admin can modify nfs options"); } if (!pool.getHypervisor().equals(HypervisorType.KVM) && !pool.getHypervisor().equals((HypervisorType.Simulator))) { @@ -1026,8 +1025,7 @@ public PrimaryDataStoreInfo createPool(CreateStoragePoolCmd cmd) throws Resource throw new InvalidParameterValueException("unable to find zone by id " + zoneId); } // Check if zone is disabled - Account account = CallContext.current().getCallingAccount(); - if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(account.getId())) { + if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !CallContext.current().isCallingAccountRootAdmin()) { throw new PermissionDeniedException(String.format("Cannot perform this operation, Zone is currently disabled: %s", zone)); } @@ -1220,7 +1218,7 @@ public PrimaryDataStoreInfo updateStoragePool(UpdateStoragePoolCmd cmd) throws I } Map inputDetails = extractApiParamAsMap(cmd.getDetails()); - checkNFSMountOptionsForUpdate(inputDetails, pool, cmd.getEntityOwnerId()); + checkNFSMountOptionsForUpdate(inputDetails, pool); String name = cmd.getName(); if(StringUtils.isNotBlank(name)) { @@ -1356,8 +1354,7 @@ private void changeStoragePoolScopeToCluster(StoragePoolVO primaryStorage, Long public void changeStoragePoolScope(ChangeStoragePoolScopeCmd cmd) throws IllegalArgumentException, InvalidParameterValueException, PermissionDeniedException { Long id = cmd.getId(); - Long accountId = cmd.getEntityOwnerId(); - if (!_accountMgr.isRootAdmin(accountId)) { + if (!CallContext.current().isCallingAccountRootAdmin()) { throw new PermissionDeniedException("Only root admin can perform this operation"); } @@ -3937,8 +3934,7 @@ public ImageStore discoverImageStore(String name, String url, String providerNam throw new InvalidParameterValueException("Can't find zone by id " + zoneId); } - Account account = CallContext.current().getCallingAccount(); - if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(account.getId())) { + if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !CallContext.current().isCallingAccountRootAdmin()) { PermissionDeniedException ex = new PermissionDeniedException("Cannot perform this operation, Zone with specified id is currently disabled"); ex.addProxyObject(zone.getUuid(), "dcId"); throw ex; @@ -4319,8 +4315,7 @@ public ImageStore createSecondaryStagingStore(CreateSecondaryStagingStoreCmd cmd throw new InvalidParameterValueException("Can't find zone by id " + dcId); } - Account account = CallContext.current().getCallingAccount(); - if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(account.getId())) { + if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !CallContext.current().isCallingAccountRootAdmin()) { PermissionDeniedException ex = new PermissionDeniedException("Cannot perform this operation, Zone with specified id is currently disabled"); ex.addProxyObject(zone.getUuid(), "dcId"); throw ex; diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 529ed3f1d7b8..30b17639fded 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -35,16 +35,11 @@ import javax.inject.Inject; -import com.cloud.projects.Project; -import com.cloud.projects.ProjectManager; -import com.cloud.vm.snapshot.VMSnapshot; -import com.cloud.vm.snapshot.VMSnapshotDetailsVO; -import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao; -import org.apache.cloudstack.api.command.user.volume.AssignVolumeCmd; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ApiErrorCode; import org.apache.cloudstack.api.InternalIdentity; import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.command.user.volume.AssignVolumeCmd; import org.apache.cloudstack.api.command.user.volume.AttachVolumeCmd; import org.apache.cloudstack.api.command.user.volume.ChangeOfferingForVolumeCmd; import org.apache.cloudstack.api.command.user.volume.CheckAndRepairVolumeCmd; @@ -164,6 +159,8 @@ import com.cloud.offering.DiskOffering; import com.cloud.org.Cluster; import com.cloud.org.Grouping; +import com.cloud.projects.Project; +import com.cloud.projects.ProjectManager; import com.cloud.resource.ResourceManager; import com.cloud.resource.ResourceState; import com.cloud.serializer.GsonHelper; @@ -215,10 +212,10 @@ import com.cloud.utils.fsm.NoTransitionException; import com.cloud.utils.fsm.StateMachine2; import com.cloud.vm.DiskProfile; -import com.cloud.vm.VMInstanceDetailVO; import com.cloud.vm.UserVmManager; import com.cloud.vm.UserVmService; import com.cloud.vm.UserVmVO; +import com.cloud.vm.VMInstanceDetailVO; import com.cloud.vm.VMInstanceVO; import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachine.State; @@ -237,10 +234,13 @@ import com.cloud.vm.VmWorkSerializer; import com.cloud.vm.VmWorkTakeVolumeSnapshot; import com.cloud.vm.dao.UserVmDao; -import com.cloud.vm.dao.VMInstanceDetailsDao; import com.cloud.vm.dao.VMInstanceDao; +import com.cloud.vm.dao.VMInstanceDetailsDao; +import com.cloud.vm.snapshot.VMSnapshot; +import com.cloud.vm.snapshot.VMSnapshotDetailsVO; import com.cloud.vm.snapshot.VMSnapshotVO; import com.cloud.vm.snapshot.dao.VMSnapshotDao; +import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao; import com.google.gson.Gson; import com.google.gson.GsonBuilder; import com.google.gson.JsonParseException; @@ -567,7 +567,7 @@ private boolean validateVolume(Account caller, long ownerId, Long zoneId, String } // Check if zone is disabled - if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller.getId())) { + if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller)) { throw new PermissionDeniedException("Cannot perform this operation, Zone is currently disabled: " + zoneId); } @@ -745,7 +745,7 @@ public VolumeVO allocVolume(CreateVolumeCmd cmd) throws ResourceAllocationExcept if (displayVolume == null) { displayVolume = true; } else { - if (!_accountMgr.isRootAdmin(caller.getId())) { + if (!CallContext.current().isCallingAccountRootAdmin()) { throw new PermissionDeniedException("Cannot update parameter displayvolume, only admin permitted "); } } @@ -942,7 +942,7 @@ public VolumeVO allocVolume(CreateVolumeCmd cmd) throws ResourceAllocationExcept } // Check if zone is disabled - if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller.getId())) { + if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller)) { throw new PermissionDeniedException(String.format("Cannot perform this operation, Zone: %s is currently disabled", zone)); } @@ -2890,7 +2890,7 @@ public Volume updateVolume(long volumeId, String path, String state, Long storag String customId, long entityOwnerId, String chainInfo, String name) { Account caller = CallContext.current().getCallingAccount(); - if (!_accountMgr.isRootAdmin(caller.getId())) { + if (!CallContext.current().isCallingAccountRootAdmin()) { if (path != null || state != null || storageId != null || displayVolume != null || customId != null || chainInfo != null) { throw new InvalidParameterValueException("The domain admin and normal user are " + "not allowed to update volume except volume name & delete protection"); @@ -3975,7 +3975,7 @@ public Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName, throw new InvalidParameterValueException(String.format("Can't find zone for the volume ID: %s", volume.getUuid())); } - if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller.getId())) { + if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !CallContext.current().isCallingAccountRootAdmin()) { throw new PermissionDeniedException("Cannot perform this operation, Zone is currently disabled: " + zone.getName()); } @@ -4031,7 +4031,7 @@ public Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName, if (dataCenter == null) { throw new InvalidParameterValueException("Unable to find the specified zone"); } - if (Grouping.AllocationState.Disabled.equals(dataCenter.getAllocationState()) && !_accountMgr.isRootAdmin(caller.getId())) { + if (Grouping.AllocationState.Disabled.equals(dataCenter.getAllocationState()) && !_accountMgr.isRootAdmin(caller)) { throw new PermissionDeniedException("Cannot perform this operation, Zone is currently disabled: " + dataCenter.getName()); } if (DataCenter.Type.Edge.equals(dataCenter.getType())) { @@ -4068,7 +4068,7 @@ public Snapshot allocSnapshotForVm(Long vmId, Long volumeId, String snapshotName throw new InvalidParameterValueException("Can't find zone by id " + volume.getDataCenterId()); } - if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller.getId())) { + if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller)) { throw new PermissionDeniedException("Cannot perform this operation, Zone is currently disabled: " + zone.getName()); } @@ -4112,7 +4112,7 @@ public String extractVolume(ExtractVolumeCmd cmd) { String mode = cmd.getMode(); Account account = CallContext.current().getCallingAccount(); - if (!_accountMgr.isRootAdmin(account.getId()) && ApiDBUtils.isExtractionDisabled()) { + if (!_accountMgr.isRootAdmin(account) && ApiDBUtils.isExtractionDisabled()) { throw new PermissionDeniedException("Extraction has been disabled by admin"); } @@ -4159,7 +4159,7 @@ public String extractVolume(ExtractVolumeCmd cmd) { // we allow extraction of all ISO based // volumes boolean isExtractable = template.isExtractable() && template.getTemplateType() != Storage.TemplateType.SYSTEM; - if (!isExtractable && account != null && !_accountMgr.isRootAdmin(account.getId())) { + if (!isExtractable && account != null && !_accountMgr.isRootAdmin(account)) { // Global admins are always allowed to extract PermissionDeniedException ex = new PermissionDeniedException("The volume with specified volumeId is not allowed to be extracted"); ex.addProxyObject(volume.getUuid(), "volumeId"); diff --git a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java index 5a4d6a68402a..d14830c2edbd 100755 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -529,7 +529,7 @@ public String extractSnapshot(ExtractSnapshotCmd cmd) { Long snapshotId = cmd.getId(); Long zoneId = cmd.getZoneId(); - if (!_accountMgr.isRootAdmin(caller.getId()) && ApiDBUtils.isExtractionDisabled()) { + if (!_accountMgr.isRootAdmin(caller) && ApiDBUtils.isExtractionDisabled()) { logger.error("Extraction is disabled through [{}].", Config.DisableExtraction); throw new PermissionDeniedException("Extraction could not be completed."); } @@ -1125,7 +1125,7 @@ protected void validatePolicyZones(List zoneIds, VolumeVO volume, Account if (DataCenter.Type.Edge.equals(zone.getType())) { throw new InvalidParameterValueException("Backing up of snapshot is not supported by the zone of the volume. Snapshots can not be taken for multiple zones"); } - boolean isRootAdminCaller = _accountMgr.isRootAdmin(caller.getId()); + boolean isRootAdminCaller = _accountMgr.isRootAdmin(caller); for (Long zoneId : zoneIds) { getCheckedDestinationZoneForSnapshotCopy(zoneId, isRootAdminCaller); } @@ -1219,7 +1219,7 @@ public SnapshotPolicyVO createPolicy(CreateSnapshotPolicyCmd cmd, Account policy if (display) { long accountLimit = _resourceLimitMgr.findCorrectResourceLimitForAccount(owner, ResourceType.snapshot, null); long domainLimit = _resourceLimitMgr.findCorrectResourceLimitForDomain(_domainMgr.getDomain(owner.getDomainId()), ResourceType.snapshot, null); - if (!_accountMgr.isRootAdmin(owner.getId()) && ((accountLimit != -1 && maxSnaps > accountLimit) || (domainLimit != -1 && maxSnaps > domainLimit))) { + if (!_accountMgr.isRootAdmin(owner) && ((accountLimit != -1 && maxSnaps > accountLimit) || (domainLimit != -1 && maxSnaps > domainLimit))) { String message = "domain/account"; if (owner.getType() == Account.Type.PROJECT) { message = "domain/project"; @@ -2135,7 +2135,7 @@ public Snapshot copySnapshot(CopySnapshotCmd cmd) throws StorageUnavailableExcep SnapshotVO snapshot = snapshotZonePair.first(); sourceZoneId = snapshotZonePair.second(); Map dataCenterVOs = new HashMap<>(); - boolean isRootAdminCaller = _accountMgr.isRootAdmin(caller.getId()); + boolean isRootAdminCaller = _accountMgr.isRootAdmin(caller); for (Long destZoneId: destZoneIds) { DataCenterVO dstZone = getCheckedDestinationZoneForSnapshotCopy(destZoneId, isRootAdminCaller); dataCenterVOs.put(destZoneId, dstZone); diff --git a/server/src/main/java/com/cloud/template/TemplateAdapterBase.java b/server/src/main/java/com/cloud/template/TemplateAdapterBase.java index 75c63aebe0d8..1c510baa8fac 100644 --- a/server/src/main/java/com/cloud/template/TemplateAdapterBase.java +++ b/server/src/main/java/com/cloud/template/TemplateAdapterBase.java @@ -320,7 +320,7 @@ public TemplateProfile prepare(boolean isIso, long userId, String name, String d sshkeyEnabled = Boolean.FALSE; } - boolean isAdmin = _accountMgr.isRootAdmin(templateOwner.getId()); + boolean isAdmin = _accountMgr.isRootAdmin(templateOwner); boolean isRegionStore = false; List stores = _imgStoreDao.findRegionImageStores(); if (stores != null && stores.size() > 0) { @@ -372,7 +372,7 @@ public TemplateProfile prepare(boolean isIso, long userId, String name, String d throw new IllegalArgumentException("Please specify a valid zone."); } Account caller = CallContext.current().getCallingAccount(); - if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller.getId())) { + if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller)) { throw new PermissionDeniedException(String.format("Cannot perform this operation, Zone %s is currently disabled", zone)); } } diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index 81b75c23eba1..ff80b75f5e10 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -362,14 +362,13 @@ public VirtualMachineTemplate registerIso(RegisterIsoCmd cmd) throws ResourceAll @Override @ActionEvent(eventType = EventTypes.EVENT_TEMPLATE_CREATE, eventDescription = "creating template") public VirtualMachineTemplate registerTemplate(RegisterTemplateCmd cmd) throws URISyntaxException, ResourceAllocationException { - Account account = CallContext.current().getCallingAccount(); if (cmd.getTemplateTag() != null) { - if (!_accountService.isRootAdmin(account.getId())) { + if (!CallContext.current().isCallingAccountRootAdmin()) { throw new PermissionDeniedException("Parameter templatetag can only be specified by a Root Admin, permission denied"); } } if (cmd.isRoutingType() != null) { - if (!_accountService.isRootAdmin(account.getId())) { + if (!CallContext.current().isCallingAccountRootAdmin()) { throw new PermissionDeniedException("Parameter isrouting can only be specified by a Root Admin, permission denied"); } } @@ -550,7 +549,8 @@ private String extract(Account caller, Long templateId, String url, Long zoneId, if (isISO) { desc = Upload.Type.ISO.toString(); } - if (!_accountMgr.isRootAdmin(caller.getId()) && ApiDBUtils.isExtractionDisabled()) { + boolean isRootAdmin = _accountMgr.isRootAdmin(caller); + if (!isRootAdmin && ApiDBUtils.isExtractionDisabled()) { throw new PermissionDeniedException("Extraction has been disabled by admin"); } @@ -577,7 +577,7 @@ private String extract(Account caller, Long templateId, String url, Long zoneId, throw new IllegalArgumentException("Please specify a valid zone."); } - if (!_accountMgr.isRootAdmin(caller.getId()) && !template.isExtractable()) { + if (!isRootAdmin && !template.isExtractable()) { throw new InvalidParameterValueException(String.format("Unable to extract template %s as it's not extractable", template)); } @@ -1815,7 +1815,7 @@ public VMTemplateVO createPrivateTemplateRecord(CreateTemplateCmd cmd, Account t } if (cmd.getTemplateTag() != null) { - if (!_accountService.isRootAdmin(caller.getId())) { + if (!_accountService.isRootAdmin(caller)) { throw new PermissionDeniedException("Parameter templatetag can only be specified by a Root Admin, permission denied"); } } @@ -2163,7 +2163,7 @@ private VMTemplateVO updateTemplateOrIso(BaseUpdateTemplateOrIsoCmd cmd) { // do a permission check _accountMgr.checkAccess(account, AccessType.OperateEntry, true, template); if (cmd.isRoutingType() != null) { - if (!_accountService.isRootAdmin(account.getId())) { + if (!_accountService.isRootAdmin(account)) { throw new PermissionDeniedException("Parameter isrouting can only be specified by a Root Admin, permission denied"); } } diff --git a/server/src/main/java/com/cloud/usage/UsageServiceImpl.java b/server/src/main/java/com/cloud/usage/UsageServiceImpl.java index c46f1f43fd07..4d16a1e15da8 100644 --- a/server/src/main/java/com/cloud/usage/UsageServiceImpl.java +++ b/server/src/main/java/com/cloud/usage/UsageServiceImpl.java @@ -193,7 +193,7 @@ public Pair, Integer> getUsageRecords(ListUsageRecordsCmd accountId = caller.getId(); //List records for all the accounts if the caller account is of type admin. //If account_id or account_name is explicitly mentioned, list records for the specified account only even if the caller is of type admin - ignoreAccountId = _accountService.isRootAdmin(caller.getId()); + ignoreAccountId = _accountService.isRootAdmin(caller); logger.debug("Account details not available. Using userContext account: {}", caller); } diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index d93993af9abd..b9f41d84a224 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -598,21 +598,26 @@ public boolean isAdmin(Long accountId) { @Override public boolean isRootAdmin(Long accountId) { if (accountId != null) { - AccountVO acct = _accountDao.findById(accountId); - if (acct == null) { - return false; //account is deleted or does not exist - } - for (SecurityChecker checker : _securityCheckers) { - try { - if (checker.checkAccess(acct, null, null, "SystemCapability")) { - if (logger.isTraceEnabled()) { - logger.trace("Root Access granted to " + acct + " by " + checker.getName()); - } - return true; + return isRootAdmin(_accountDao.findById(accountId)); + } + return false; + } + + @Override + public boolean isRootAdmin(Account account) { + if (account == null) { + return false; //account is deleted or does not exist + } + for (SecurityChecker checker : _securityCheckers) { + try { + if (checker.checkAccess(account, null, null, "SystemCapability")) { + if (logger.isTraceEnabled()) { + logger.trace("Root Access granted to " + account + " by " + checker.getName()); } - } catch (PermissionDeniedException ex) { - return false; + return true; } + } catch (PermissionDeniedException ex) { + return false; } } return false; @@ -678,10 +683,7 @@ public boolean isInternalAccount(long accountId) { if (account == null) { return false; //account is deleted or does not exist } - if (isRootAdmin(accountId) || (account.getType() == Account.Type.ADMIN)) { - return true; - } - return false; + return isRootAdmin(account) || (account.getType() == Account.Type.ADMIN); } @Override @@ -719,7 +721,7 @@ public void checkAccess(Account caller, AccessType accessType, boolean sameOwner } } - if (caller.getId() == Account.ACCOUNT_ID_SYSTEM || isRootAdmin(caller.getId())) { + if (caller.getId() == Account.ACCOUNT_ID_SYSTEM || isRootAdmin(caller)) { // no need to make permission checks if the system/root admin makes the call if (logger.isTraceEnabled()) { logger.trace("No need to make permission check for System/RootAdmin account, returning true"); @@ -1644,11 +1646,12 @@ protected void checkCallerRoleTypeAllowedForUserOrAccountOperations(Account user protected void checkCallerApiPermissionsForUserOrAccountOperations(Account userAccount) { Account callingAccount = getCurrentCallingAccount(); - boolean isCallerRootAdmin = callingAccount.getId() == Account.ACCOUNT_ID_SYSTEM || isRootAdmin(callingAccount.getId()); + boolean isCallerRootAdmin = callingAccount.getId() == Account.ACCOUNT_ID_SYSTEM || + CallContext.current().isCallingAccountRootAdmin(); if (isCallerRootAdmin) { logger.trace(String.format("Admin account [%s, %s] performing this operation for user account [%s, %s] ", callingAccount.getName(), callingAccount.getUuid(), userAccount.getName(), userAccount.getUuid())); - } else if (isRootAdmin(userAccount.getAccountId())) { + } else if (isRootAdmin(userAccount)) { String errMsg = String.format("Account [%s, %s] cannot perform this operation for user account [%s, %s] ", callingAccount.getName(), callingAccount.getUuid(), userAccount.getName(), userAccount.getUuid()); logger.error(errMsg); throw new PermissionDeniedException(errMsg); @@ -1690,7 +1693,8 @@ public void validateUserPasswordAndUpdateIfNeeded(String newPassword, UserVO use passwordPolicy.verifyIfPasswordCompliesWithPasswordPolicies(newPassword, user.getUsername(), getAccount(user.getAccountId()).getDomainId()); Account callingAccount = getCurrentCallingAccount(); - boolean isRootAdminExecutingPasswordUpdate = callingAccount.getId() == Account.ACCOUNT_ID_SYSTEM || isRootAdmin(callingAccount.getId()); + boolean isRootAdminExecutingPasswordUpdate = callingAccount.getId() == Account.ACCOUNT_ID_SYSTEM || + CallContext.current().isCallingAccountRootAdmin(); boolean isDomainAdmin = isDomainAdmin(callingAccount.getId()); boolean isAdmin = isDomainAdmin || isRootAdminExecutingPasswordUpdate; boolean skipValidation = isAdmin || skipCurrentPassValidation; @@ -3076,19 +3080,6 @@ public Pair> getKeys(Long userId) { return new Pair<>(apiKeyAccess, keys); } - protected void preventRootDomainAdminAccessToRootAdminKeys(User caller, ControlledEntity account) { - if (isDomainAdminForRootDomain(caller) && isRootAdmin(account.getAccountId())) { - String msg = String.format("Caller Username %s does not have access to root admin keys", caller.getUsername()); - logger.error(msg); - throw new PermissionDeniedException(msg); - } - } - - protected boolean isDomainAdminForRootDomain(User callingUser) { - AccountVO caller = _accountDao.findById(callingUser.getAccountId()); - return caller.getType() == Account.Type.DOMAIN_ADMIN && caller.getDomainId() == Domain.ROOT_DOMAIN; - } - @Override public List listUserTwoFactorAuthenticationProviders() { return userTwoFactorAuthenticationProviders; diff --git a/server/src/main/java/com/cloud/user/DomainManagerImpl.java b/server/src/main/java/com/cloud/user/DomainManagerImpl.java index 6fc9c6f5ef53..7134790f76ed 100644 --- a/server/src/main/java/com/cloud/user/DomainManagerImpl.java +++ b/server/src/main/java/com/cloud/user/DomainManagerImpl.java @@ -721,7 +721,7 @@ public Pair, Integer> searchForDomains(ListDomainsCmd cmd } _accountMgr.checkAccess(caller, domain); } else { - if (!_accountMgr.isRootAdmin(caller.getId())) { + if (!_accountMgr.isRootAdmin(caller)) { domainId = caller.getDomainId(); } if (listAll) { diff --git a/server/src/main/java/com/cloud/uuididentity/UUIDManagerImpl.java b/server/src/main/java/com/cloud/uuididentity/UUIDManagerImpl.java index 9b6ac8a960a6..a5afeeeb01f4 100644 --- a/server/src/main/java/com/cloud/uuididentity/UUIDManagerImpl.java +++ b/server/src/main/java/com/cloud/uuididentity/UUIDManagerImpl.java @@ -50,7 +50,7 @@ public void checkUuid(String uuid, Class entityType) { Account caller = CallContext.current().getCallingAccount(); // Only admin and system allowed to do this - if (!(caller.getId() == Account.ACCOUNT_ID_SYSTEM || _accountMgr.isRootAdmin(caller.getId()))) { + if (!(caller.getId() == Account.ACCOUNT_ID_SYSTEM || _accountMgr.isRootAdmin(caller))) { throw new PermissionDeniedException("Please check your permissions, you are not allowed to create/update custom id"); } diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 6cdcba7e3b62..c3e03312b664 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -4145,7 +4145,7 @@ private UserVm createVirtualMachine(DataCenter zone, ServiceOffering serviceOffe assert !(requestedIps != null && (defaultIps.getIp4Address() != null || defaultIps.getIp6Address() != null)) : "requestedIp list and defaultNetworkIp should never be specified together"; if (Grouping.AllocationState.Disabled == zone.getAllocationState() - && !_accountMgr.isRootAdmin(caller.getId())) { + && !_accountMgr.isRootAdmin(caller)) { throw new PermissionDeniedException( String.format("Cannot perform this operation, Zone is currently disabled: %s", zone)); } @@ -4360,7 +4360,7 @@ private UserVm getUncheckedUserVmResource(DataCenter zone, String hostName, Stri // Root admin has access to both VM and AG by default, // but // make sure the owner of these entities is same - if (caller.getId() == Account.ACCOUNT_ID_SYSTEM || _accountMgr.isRootAdmin(caller.getId())) { + if (caller.getId() == Account.ACCOUNT_ID_SYSTEM || _accountMgr.isRootAdmin(caller)) { if (!_affinityGroupService.isAffinityGroupAvailableInDomain(ag.getId(), owner.getDomainId())) { throw new PermissionDeniedException("Affinity Group " + ag + " does not belong to the VM's domain"); } @@ -4370,7 +4370,7 @@ private UserVm getUncheckedUserVmResource(DataCenter zone, String hostName, Stri // Root admin has access to both VM and AG by default, // but // make sure the owner of these entities is same - if (caller.getId() == Account.ACCOUNT_ID_SYSTEM || _accountMgr.isRootAdmin(caller.getId())) { + if (caller.getId() == Account.ACCOUNT_ID_SYSTEM || _accountMgr.isRootAdmin(caller)) { if (ag.getAccountId() != owner.getAccountId()) { throw new PermissionDeniedException("Affinity Group " + ag + " does not belong to the VM's account"); } @@ -5678,7 +5678,7 @@ public Pair> startVirtualMach // Choose deployment planner // Host takes 1st preference, Cluster takes 2nd preference and Pod takes 3rd // Default behaviour is invoked when host, cluster or pod are not specified - boolean isRootAdmin = _accountService.isRootAdmin(callerAccount.getId()); + boolean isRootAdmin = _accountService.isRootAdmin(callerAccount); Pod destinationPod = getDestinationPod(podId, isRootAdmin); Cluster destinationCluster = getDestinationCluster(clusterId, isRootAdmin); HostVO destinationHost = getDestinationHost(hostId, isRootAdmin, isExplicitHost); @@ -6388,9 +6388,7 @@ private UserVm createVirtualMachine(BaseDeployVMCmd cmd, DataCenter zone, Accoun } Account caller = CallContext.current().getCallingAccount(); - Long callerId = caller.getId(); - - boolean isRootAdmin = _accountService.isRootAdmin(callerId); + boolean isRootAdmin = CallContext.current().isCallingAccountRootAdmin(); Long hostId = cmd.getHostId(); getDestinationHost(hostId, isRootAdmin, true); @@ -6450,7 +6448,7 @@ private UserVm createVirtualMachine(BaseDeployVMCmd cmd, DataCenter zone, Accoun // Add extraConfig to vm_instance_details table String extraConfig = cmd.getExtraConfig(); if (StringUtils.isNotBlank(extraConfig)) { - if (EnableAdditionalVmConfig.valueIn(callerId)) { + if (EnableAdditionalVmConfig.valueIn(caller.getId())) { logger.info("Adding extra configuration to user vm: {}", vm); addExtraConfig(vm, extraConfig); } else { @@ -6933,7 +6931,7 @@ public VirtualMachine getVm(long vmId) { private VMInstanceVO preVmStorageMigrationCheck(Long vmId) { // access check - only root admin can migrate VM Account caller = CallContext.current().getCallingAccount(); - if (!_accountMgr.isRootAdmin(caller.getId())) { + if (!_accountMgr.isRootAdmin(caller)) { if (logger.isDebugEnabled()) { logger.debug("Caller is not a root admin, permission denied to migrate the VM"); } @@ -7075,7 +7073,7 @@ public VirtualMachine migrateVirtualMachine(Long vmId, Host destinationHost) thr VirtualMachineMigrationException { // access check - only root admin can migrate VM Account caller = CallContext.current().getCallingAccount(); - if (!_accountMgr.isRootAdmin(caller.getId())) { + if (!_accountMgr.isRootAdmin(caller)) { if (logger.isDebugEnabled()) { logger.debug("Caller is not a root admin, permission denied to migrate the VM"); } @@ -7688,7 +7686,7 @@ public VirtualMachine migrateVirtualMachineWithVolume(Long vmId, Host destinatio ConcurrentOperationException, ManagementServerException, VirtualMachineMigrationException { // Access check - only root administrator can migrate VM. Account caller = CallContext.current().getCallingAccount(); - if (!_accountMgr.isRootAdmin(caller.getId())) { + if (!_accountMgr.isRootAdmin(caller)) { if (logger.isDebugEnabled()) { logger.debug("Caller is not a root admin, permission denied to migrate the VM"); } @@ -7794,7 +7792,7 @@ public UserVm moveVmToUser(final AssignVMCmd cmd) throws ResourceAllocationExcep Account caller = CallContext.current().getCallingAccount(); Long callerId = caller.getId(); logger.trace("Verifying if caller [{}] is root or domain admin.", caller); - if (!_accountMgr.isRootAdmin(callerId) && !_accountMgr.isDomainAdmin(callerId)) { + if (!CallContext.current().isCallingAccountRootAdmin() && !_accountMgr.isDomainAdmin(callerId)) { throw new InvalidParameterValueException(String.format("Only root or domain admins are allowed to assign VMs. Caller [%s] is of type [%s].", caller, caller.getType())); } diff --git a/server/src/main/java/org/apache/cloudstack/acl/ProjectRoleManagerImpl.java b/server/src/main/java/org/apache/cloudstack/acl/ProjectRoleManagerImpl.java index 91bbb349a07e..427c73f0fd37 100644 --- a/server/src/main/java/org/apache/cloudstack/acl/ProjectRoleManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/acl/ProjectRoleManagerImpl.java @@ -23,9 +23,9 @@ import javax.inject.Inject; +import org.apache.cloudstack.acl.RolePermissionEntity.Permission; import org.apache.cloudstack.acl.dao.ProjectRoleDao; import org.apache.cloudstack.acl.dao.ProjectRolePermissionsDao; -import org.apache.cloudstack.acl.RolePermissionEntity.Permission; import org.apache.cloudstack.api.command.admin.acl.project.CreateProjectRoleCmd; import org.apache.cloudstack.api.command.admin.acl.project.CreateProjectRolePermissionCmd; import org.apache.cloudstack.api.command.admin.acl.project.DeleteProjectRoleCmd; @@ -46,7 +46,6 @@ import com.cloud.projects.dao.ProjectDao; import com.cloud.user.Account; import com.cloud.user.AccountService; -import com.cloud.user.User; import com.cloud.user.dao.AccountDao; import com.cloud.utils.ListUtils; import com.cloud.utils.component.ManagerBase; @@ -87,18 +86,18 @@ private void checkAccess(Long projectId) { throw new PermissionDeniedException("Dynamic api checker is not enabled, aborting role operation"); } - User user = getCurrentUser(); - Account callerAcc = accountDao.findById(user.getAccountId()); + Account callerAcc = CallContext.current().getCallingAccount(); if (callerAcc == null || callerAcc.getRoleId() == null) { throw new PermissionDeniedException("Restricted API called by an invalid user account"); } - if (accountService.isRootAdmin(callerAcc.getId()) || accountService.isDomainAdmin(callerAcc.getAccountId())) { + if (accountService.isRootAdmin(callerAcc) || accountService.isDomainAdmin(callerAcc.getAccountId())) { return; } - ProjectAccount projectAccount = projAccDao.findByProjectIdUserId(projectId, callerAcc.getAccountId(), user.getId()); + ProjectAccount projectAccount = projAccDao.findByProjectIdUserId(projectId, callerAcc.getAccountId(), + CallContext.current().getCallingUserId()); if (projectAccount == null) { projectAccount = projAccDao.findByProjectIdAccountId(projectId, callerAcc.getAccountId()); if (projectAccount == null) { @@ -280,22 +279,6 @@ public Boolean doInTransaction(TransactionStatus status) { }); } - protected Account getCurrentAccount() { - return CallContext.current().getCallingAccount(); - } - - private Long getProjectIdOfAccount() { - Project project = projectDao.findByProjectAccountId(getCurrentAccount().getAccountId()); - if (project != null) { - return project.getId(); - } - return null; - } - - protected User getCurrentUser() { - return CallContext.current().getCallingUser(); - } - @Override public List> getCommands() { List> cmdList = new ArrayList>(); diff --git a/server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java b/server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java index d1ae1b44a51f..199aef094c54 100644 --- a/server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java @@ -80,7 +80,7 @@ public void checkCallerAccess() { if (!isEnabled()) { throw new PermissionDeniedException("Dynamic api checker is not enabled, aborting role operation"); } - Account caller = getCurrentAccount(); + Account caller = CallContext.current().getCallingAccount(); if (caller == null || caller.getRoleId() == null) { throw new PermissionDeniedException("Restricted API called by an invalid user account"); } @@ -141,14 +141,6 @@ public Role findRole(Long id) { return findRole(id, false); } - /** - * Simple call to {@link CallContext#current()} to retrieve the current calling account. - * This method facilitates unit testing, it avoids mocking static methods. - */ - protected Account getCurrentAccount() { - return CallContext.current().getCallingAccount(); - } - @Override public RolePermission findRolePermission(final Long id) { if (id == null) { @@ -454,7 +446,7 @@ protected int removeRolesIfNeeded(List roles) { return 0; } - Long callerRoleId = getCurrentAccount().getRoleId(); + Long callerRoleId = CallContext.current().getCallingAccount().getRoleId(); Map callerRolePermissions = getRoleRulesAndPermissions(callerRoleId); int count = 0; @@ -572,7 +564,7 @@ public List findAllPermissionsBy(final Long roleId) { } private boolean isCallerRootAdmin() { - return accountManager.isRootAdmin(getCurrentAccount().getId()); + return CallContext.current().isCallingAccountRootAdmin(); } @Override diff --git a/server/src/main/java/org/apache/cloudstack/affinity/AffinityGroupServiceImpl.java b/server/src/main/java/org/apache/cloudstack/affinity/AffinityGroupServiceImpl.java index 650d52f9dc5d..ffdf95e77198 100644 --- a/server/src/main/java/org/apache/cloudstack/affinity/AffinityGroupServiceImpl.java +++ b/server/src/main/java/org/apache/cloudstack/affinity/AffinityGroupServiceImpl.java @@ -133,7 +133,7 @@ public AffinityGroup createAffinityGroup(final String accountName, final Long pr } Account caller = CallContext.current().getCallingAccount(); - if (processor.isAdminControlledGroup() && !_accountMgr.isRootAdmin(caller.getId())) { + if (processor.isAdminControlledGroup() && !_accountMgr.isRootAdmin(caller)) { throw new PermissionDeniedException("Cannot create the affinity group"); } @@ -169,7 +169,7 @@ public AffinityGroup createAffinityGroup(final String accountName, final Long pr } private void verifyAccessToDomainWideProcessor(Account caller, AffinityGroupProcessor processor) { - if (!_accountMgr.isRootAdmin(caller.getId())) { + if (!_accountMgr.isRootAdmin(caller)) { throw new InvalidParameterValueException("Unable to create affinity group, account name must be passed with the domainId"); } if (!processor.canBeSharedDomainWide()) { @@ -464,7 +464,7 @@ public UserVm updateVMAffinityGroups(Long vmId, List affinityGroupIds) { if (ag.getAclType() == ACLType.Domain) { _accountMgr.checkAccess(caller, null, false, owner, ag); // make sure the affinity group is available in that domain - if (caller.getId() == Account.ACCOUNT_ID_SYSTEM || _accountMgr.isRootAdmin(caller.getId())) { + if (caller.getId() == Account.ACCOUNT_ID_SYSTEM || _accountMgr.isRootAdmin(caller)) { if (!isAffinityGroupAvailableInDomain(ag.getId(), owner.getDomainId())) { throw new PermissionDeniedException("Affinity Group " + ag + " does not belong to the VM's domain"); } @@ -474,7 +474,7 @@ public UserVm updateVMAffinityGroups(Long vmId, List affinityGroupIds) { // Root admin has access to both VM and AG by default, // but // make sure the owner of these entities is same - if (caller.getId() == Account.ACCOUNT_ID_SYSTEM || _accountMgr.isRootAdmin(caller.getId())) { + if (caller.getId() == Account.ACCOUNT_ID_SYSTEM || _accountMgr.isRootAdmin(caller)) { if (ag.getAccountId() != owner.getAccountId()) { throw new PermissionDeniedException("Affinity Group " + ag + " does not belong to the VM's account"); } diff --git a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java index 6d4d58dde145..f67d020dc71c 100644 --- a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java @@ -254,8 +254,7 @@ public List listBackupProviderOfferings(final Long zoneId) { throw new CloudRuntimeException("Invalid zone ID passed"); } validateBackupForZone(zoneId); - final Account account = CallContext.current().getCallingAccount(); - if (!accountService.isRootAdmin(account.getId())) { + if (!CallContext.current().isCallingAccountRootAdmin()) { throw new PermissionDeniedException("Parameter external can only be specified by a Root Admin, permission denied"); } final BackupProvider backupProvider = getBackupProvider(zoneId); @@ -629,7 +628,7 @@ protected int validateAndGetDefaultBackupRetentionIfRequired(Integer maxBackups, long domainLimit = resourceLimitMgr.findCorrectResourceLimitForDomain(domainManager.getDomain(owner.getDomainId()), Resource.ResourceType.backup, null); boolean exceededDomainLimit = domainLimit != -1 && maxBackups > domainLimit; - if (!accountManager.isRootAdmin(owner.getId()) && (exceededAccountLimit || exceededDomainLimit)) { + if (!accountManager.isRootAdmin(owner) && (exceededAccountLimit || exceededDomainLimit)) { throw new InvalidParameterValueException( String.format("'maxbackups' should not exceed the domain/%s backup limit.", owner.getType() == Account.Type.PROJECT ? "project" : "account") ); diff --git a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java index 306023a22638..8ffbb62f8d58 100644 --- a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java @@ -200,8 +200,7 @@ public ListResponse listConsoleSessions(ListConsoleSessi ListResponse response = new ListResponse<>(); ResponseObject.ResponseView responseView = ResponseObject.ResponseView.Restricted; - Long callerId = CallContext.current().getCallingAccountId(); - if (accountManager.isRootAdmin(callerId)) { + if (CallContext.current().isCallingAccountRootAdmin()) { responseView = ResponseObject.ResponseView.Full; } @@ -344,7 +343,7 @@ public void acquireSession(String sessionUuid, String clientAddress) { } protected boolean checkSessionPermission(VirtualMachine vm, Account account) { - if (accountManager.isRootAdmin(account.getId())) { + if (accountManager.isRootAdmin(account)) { return true; } diff --git a/server/src/main/java/org/apache/cloudstack/storage/sharedfs/SharedFSServiceImpl.java b/server/src/main/java/org/apache/cloudstack/storage/sharedfs/SharedFSServiceImpl.java index 4f0aabd3f379..f186ee7af9d7 100644 --- a/server/src/main/java/org/apache/cloudstack/storage/sharedfs/SharedFSServiceImpl.java +++ b/server/src/main/java/org/apache/cloudstack/storage/sharedfs/SharedFSServiceImpl.java @@ -515,8 +515,7 @@ public ListResponse searchForSharedFS(ResponseObject.ResponseV return response; } - Account caller = CallContext.current().getCallingAccount(); - if (accountMgr.isRootAdmin(caller.getId())) { + if (CallContext.current().isCallingAccountRootAdmin()) { respView = ResponseObject.ResponseView.Full; } diff --git a/server/src/test/java/com/cloud/acl/DomainCheckerTest.java b/server/src/test/java/com/cloud/acl/DomainCheckerTest.java index a5ec41306d85..6d206d6b8cf3 100644 --- a/server/src/test/java/com/cloud/acl/DomainCheckerTest.java +++ b/server/src/test/java/com/cloud/acl/DomainCheckerTest.java @@ -61,9 +61,8 @@ private ControlledEntity getMockedEntity(long accountId) { @Test public void testRootAdminHasAccess() { Account rootAdmin = Mockito.mock(Account.class); - Mockito.when(rootAdmin.getId()).thenReturn(1L); ControlledEntity entity = getMockedEntity(2L); - Mockito.when(_accountService.isRootAdmin(rootAdmin.getId())).thenReturn(true); + Mockito.when(_accountService.isRootAdmin(rootAdmin)).thenReturn(true); domainChecker.validateCallerHasAccessToEntityOwner(rootAdmin, entity, SecurityChecker.AccessType.ModifyProject); } diff --git a/server/src/test/java/com/cloud/api/ApiResponseHelperTest.java b/server/src/test/java/com/cloud/api/ApiResponseHelperTest.java index 223b0740cf27..60339ae7a41f 100644 --- a/server/src/test/java/com/cloud/api/ApiResponseHelperTest.java +++ b/server/src/test/java/com/cloud/api/ApiResponseHelperTest.java @@ -38,8 +38,10 @@ import java.util.UUID; import org.apache.cloudstack.annotation.dao.AnnotationDao; +import org.apache.cloudstack.api.ResponseObject; import org.apache.cloudstack.api.response.AutoScaleVmGroupResponse; import org.apache.cloudstack.api.response.AutoScaleVmProfileResponse; +import org.apache.cloudstack.api.response.ConsoleSessionResponse; import org.apache.cloudstack.api.response.DirectDownloadCertificateResponse; import org.apache.cloudstack.api.response.GuestOSCategoryResponse; import org.apache.cloudstack.api.response.IpQuarantineResponse; @@ -97,8 +99,6 @@ import com.cloud.vm.ConsoleSessionVO; import com.cloud.vm.NicSecondaryIp; import com.cloud.vm.VMInstanceVO; -import org.apache.cloudstack.api.ResponseObject; -import org.apache.cloudstack.api.response.ConsoleSessionResponse; @RunWith(MockitoJUnitRunner.class) public class ApiResponseHelperTest { @@ -309,7 +309,9 @@ public void testHandleCertificateResponse() { public void testAutoScaleVmGroupResponse() { AutoScaleVmGroupVO vmGroup = new AutoScaleVmGroupVO(1L, 2L, 3L, 4L, "test", 5, 6, 7, 8, new Date(), 9L, AutoScaleVmGroup.State.ENABLED); - try (MockedStatic ignored = Mockito.mockStatic(ApiDBUtils.class)) { + try (MockedStatic ignored = Mockito.mockStatic(ApiDBUtils.class); + MockedStatic callContextMockedStatic = Mockito.mockStatic(CallContext.class)) { + callContextMockedStatic.when(CallContext::current).thenReturn(Mockito.mock(CallContext.class)); when(ApiDBUtils.findAutoScaleVmProfileById(anyLong())).thenReturn(null); when(ApiDBUtils.findLoadBalancerById(anyLong())).thenReturn(null); when(ApiDBUtils.findAccountById(anyLong())).thenReturn(new AccountVO()); @@ -341,7 +343,9 @@ public void testAutoScaleVmGroupResponseWithNetwork() { "testnetwork", "displaytext", "networkdomain", null, 1L, null, null, false, null, false); IPAddressVO ipAddressVO = new IPAddressVO(new Ip("10.10.10.10"), 1L, 1L, 1L,false); - try (MockedStatic ignored = Mockito.mockStatic(ApiDBUtils.class)) { + try (MockedStatic ignored = Mockito.mockStatic(ApiDBUtils.class); + MockedStatic callContextMockedStatic = Mockito.mockStatic(CallContext.class)) { + callContextMockedStatic.when(CallContext::current).thenReturn(Mockito.mock(CallContext.class)); when(ApiDBUtils.findAutoScaleVmProfileById(anyLong())).thenReturn(null); when(ApiDBUtils.findAccountById(anyLong())).thenReturn(new AccountVO()); when(ApiDBUtils.findDomainById(anyLong())).thenReturn(new DomainVO()); diff --git a/server/src/test/java/com/cloud/api/query/QueryManagerImplTest.java b/server/src/test/java/com/cloud/api/query/QueryManagerImplTest.java index 892cd1e7def6..7da491265ef9 100644 --- a/server/src/test/java/com/cloud/api/query/QueryManagerImplTest.java +++ b/server/src/test/java/com/cloud/api/query/QueryManagerImplTest.java @@ -196,7 +196,7 @@ private void setupCommonMocks() { user = new UserVO(1, "testuser", "password", "firstname", "lastName", "email", "timezone", UUID.randomUUID().toString(), User.Source.UNKNOWN); CallContext.register(user, account); - Mockito.when(accountManager.isRootAdmin(account.getId())).thenReturn(false); + Mockito.when(accountManager.isRootAdmin(account)).thenReturn(false); final SearchBuilder eventSearchBuilder = mock(SearchBuilder.class); final SearchCriteria eventSearchCriteria = mock(SearchCriteria.class); final EventVO eventVO = mock(EventVO.class); diff --git a/server/src/test/java/com/cloud/api/query/dao/UserVmJoinDaoImplTest.java b/server/src/test/java/com/cloud/api/query/dao/UserVmJoinDaoImplTest.java index 14074add021d..ff7a43cd0a40 100755 --- a/server/src/test/java/com/cloud/api/query/dao/UserVmJoinDaoImplTest.java +++ b/server/src/test/java/com/cloud/api/query/dao/UserVmJoinDaoImplTest.java @@ -16,7 +16,6 @@ // under the License. package com.cloud.api.query.dao; -import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.MockitoAnnotations.openMocks; import java.util.Arrays; @@ -116,8 +115,7 @@ private void prepareNewUserVmResponseForVnfAppliance() { Mockito.when(userVmMock.getTemplateType()).thenReturn(Storage.TemplateType.VNF); Mockito.when(userVmMock.getTemplateFormat()).thenReturn(Storage.ImageFormat.OVA); - Mockito.when(caller.getId()).thenReturn(2L); - Mockito.when(accountMgr.isRootAdmin(nullable(Long.class))).thenReturn(true); + Mockito.when(accountMgr.isRootAdmin(Mockito.any(Account.class))).thenReturn(true); SearchBuilder searchBuilderMock = Mockito.mock(SearchBuilder.class); Mockito.doReturn(searchBuilderMock).when(userStatsDao).createSearchBuilder(); diff --git a/server/src/test/java/com/cloud/deploy/DeploymentPlanningManagerImplTest.java b/server/src/test/java/com/cloud/deploy/DeploymentPlanningManagerImplTest.java index 5b03260d2d66..d2e4dc77c518 100644 --- a/server/src/test/java/com/cloud/deploy/DeploymentPlanningManagerImplTest.java +++ b/server/src/test/java/com/cloud/deploy/DeploymentPlanningManagerImplTest.java @@ -791,7 +791,7 @@ public void findSuitablePoolsForVolumesTest() throws Exception { Account account = Mockito.mock(Account.class); Mockito.when(account.getId()).thenReturn(1L); Mockito.when(vmProfile.getOwner()).thenReturn(account); - Mockito.when(_accountMgr.isRootAdmin(account.getId())).thenReturn(Boolean.FALSE); + Mockito.when(_accountMgr.isRootAdmin(account)).thenReturn(Boolean.FALSE); Mockito.when(_dcDao.findById(dataCenterId)).thenReturn(dc); Mockito.when(dc.getAllocationState()).thenReturn(AllocationState.Enabled); diff --git a/server/src/test/java/com/cloud/network/NetworkServiceImplTest.java b/server/src/test/java/com/cloud/network/NetworkServiceImplTest.java index 2189b4517614..a3db0d8ea19e 100644 --- a/server/src/test/java/com/cloud/network/NetworkServiceImplTest.java +++ b/server/src/test/java/com/cloud/network/NetworkServiceImplTest.java @@ -327,12 +327,12 @@ public void setup() throws Exception { accountMock = Mockito.mock(Account.class); Mockito.when(service._accountMgr.finalizeOwner(any(Account.class), nullable(String.class), nullable(Long.class), nullable(Long.class))).thenReturn(accountMock); Mockito.when(callContextMock.getCallingAccount()).thenReturn(accountMock); + Mockito.when(callContextMock.isCallingAccountRootAdmin()).thenReturn(true); NetworkOffering networkOffering = Mockito.mock(NetworkOffering.class); Mockito.when(entityMgr.findById(NetworkOffering.class, 1L)).thenReturn(networkOffering); Mockito.when(networkOfferingDao.findById(1L)).thenReturn(offering); Mockito.when(physicalNetworkDao.findById(Mockito.anyLong())).thenReturn(phyNet); Mockito.when(_dcDao.findById(Mockito.anyLong())).thenReturn(dc); - Mockito.when(accountManager.isRootAdmin(accountMock.getId())).thenReturn(true); } @After diff --git a/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java b/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java index a968a2da0b7d..935f2963175e 100644 --- a/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java +++ b/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java @@ -420,9 +420,8 @@ public void testAddTaggedResourceLimits() { limits.add(Mockito.mock(ResourceLimitVO.class)); int size = limits.size(); AccountVO account = Mockito.mock(AccountVO.class); - Mockito.when(account.getId()).thenReturn(1L); Mockito.when(accountDao.findById(1L)).thenReturn(account); - Mockito.when(accountManager.isRootAdmin(1L)).thenReturn(true); + Mockito.when(accountManager.isRootAdmin(Mockito.any(Account.class))).thenReturn(true); resourceLimitManager.addTaggedResourceLimits(limits, List.of(Resource.ResourceType.cpu), hostTags, Resource.ResourceOwnerType.Account, 1L); Assert.assertEquals(size + hostTags.size(), limits.size()); } @@ -431,11 +430,11 @@ public void testAddTaggedResourceLimits() { public void testFindCorrectResourceLimitForAccount() { AccountVO account = Mockito.mock(AccountVO.class); Mockito.when(account.getId()).thenReturn(1L); - Mockito.when(accountManager.isRootAdmin(1L)).thenReturn(true); + Mockito.when(accountManager.isRootAdmin(account)).thenReturn(true); long result = resourceLimitManager.findCorrectResourceLimitForAccount(account, Resource.ResourceType.cpu, hostTags.get(0)); Assert.assertEquals(Resource.RESOURCE_UNLIMITED, result); - Mockito.when(accountManager.isRootAdmin(1L)).thenReturn(false); + Mockito.when(accountManager.isRootAdmin(account)).thenReturn(false); ResourceLimitVO limit = new ResourceLimitVO(); limit.setMax(10L); Mockito.when(resourceLimitDao.findByOwnerIdAndTypeAndTag(1L, Resource.ResourceOwnerType.Account, Resource.ResourceType.cpu, hostTags.get(0))).thenReturn(limit); @@ -456,32 +455,32 @@ public void testFindCorrectResourceLimitForAccount() { @Test public void testFindCorrectResourceLimitForAccountProjects() { - AccountVO account = Mockito.mock(AccountVO.class); - Mockito.when(account.getId()).thenReturn(1L); - Mockito.when(accountManager.isRootAdmin(1L)).thenReturn(true); + AccountVO account = Mockito.mock(AccountVO.class); + Mockito.when(account.getId()).thenReturn(1L); + Mockito.when(accountManager.isRootAdmin(account)).thenReturn(true); - long result = resourceLimitManager.findCorrectResourceLimitForAccount(account, - Resource.ResourceType.project, hostTags.get(0)); - Assert.assertEquals(Resource.RESOURCE_UNLIMITED, result); + long result = resourceLimitManager.findCorrectResourceLimitForAccount(account, + Resource.ResourceType.project, hostTags.get(0)); + Assert.assertEquals(Resource.RESOURCE_UNLIMITED, result); - Mockito.when(accountManager.isRootAdmin(1L)).thenReturn(false); - ResourceLimitVO limit = new ResourceLimitVO(); - limit.setMax(10L); - Mockito.when(resourceLimitDao.findByOwnerIdAndTypeAndTag(1L, Resource.ResourceOwnerType.Account, - Resource.ResourceType.project, hostTags.get(0))).thenReturn(limit); - result = resourceLimitManager.findCorrectResourceLimitForAccount(account, Resource.ResourceType.project, - hostTags.get(0)); - Assert.assertEquals(10L, result); + Mockito.when(accountManager.isRootAdmin(account)).thenReturn(false); + ResourceLimitVO limit = new ResourceLimitVO(); + limit.setMax(10L); + Mockito.when(resourceLimitDao.findByOwnerIdAndTypeAndTag(1L, Resource.ResourceOwnerType.Account, + Resource.ResourceType.project, hostTags.get(0))).thenReturn(limit); + result = resourceLimitManager.findCorrectResourceLimitForAccount(account, Resource.ResourceType.project, + hostTags.get(0)); + Assert.assertEquals(10L, result); - long defaultAccountProjectsMax = 15L; - Map accountResourceLimitMap = new HashMap<>(); - accountResourceLimitMap.put(Resource.ResourceType.project.name(), defaultAccountProjectsMax); - resourceLimitManager.accountResourceLimitMap = accountResourceLimitMap; - Mockito.when(resourceLimitDao.findByOwnerIdAndTypeAndTag(1L, Resource.ResourceOwnerType.Account, - Resource.ResourceType.project, hostTags.get(0))).thenReturn(null); - result = resourceLimitManager.findCorrectResourceLimitForAccount(account, Resource.ResourceType.project, - hostTags.get(0)); - Assert.assertEquals(defaultAccountProjectsMax, result); + long defaultAccountProjectsMax = 15L; + Map accountResourceLimitMap = new HashMap<>(); + accountResourceLimitMap.put(Resource.ResourceType.project.name(), defaultAccountProjectsMax); + resourceLimitManager.accountResourceLimitMap = accountResourceLimitMap; + Mockito.when(resourceLimitDao.findByOwnerIdAndTypeAndTag(1L, Resource.ResourceOwnerType.Account, + Resource.ResourceType.project, hostTags.get(0))).thenReturn(null); + result = resourceLimitManager.findCorrectResourceLimitForAccount(account, Resource.ResourceType.project, + hostTags.get(0)); + Assert.assertEquals(defaultAccountProjectsMax, result); } @Test @@ -610,8 +609,7 @@ public void testDefaultDomainProjectLimit() { @Test public void testCheckResourceLimitWithTag() { AccountVO account = Mockito.mock(AccountVO.class); - Mockito.when(account.getId()).thenReturn(1L); - Mockito.when(accountManager.isRootAdmin(1L)).thenReturn(true); + Mockito.when(accountManager.isRootAdmin(account)).thenReturn(true); try { resourceLimitManager.checkResourceLimitWithTag(account, Resource.ResourceType.cpu, hostTags.get(0), 1); } catch (ResourceAllocationException e) { @@ -622,8 +620,7 @@ public void testCheckResourceLimitWithTag() { @Test public void testCheckResourceLimitWithTagNonAdmin() throws ResourceAllocationException { AccountVO account = Mockito.mock(AccountVO.class); - Mockito.when(account.getId()).thenReturn(1L); - Mockito.when(accountManager.isRootAdmin(1L)).thenReturn(false); + Mockito.when(accountManager.isRootAdmin(account)).thenReturn(false); Mockito.doReturn(new ArrayList()).when(resourceLimitManager).lockAccountAndOwnerDomainRows(Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyString()); Mockito.doNothing().when(resourceLimitManager).checkAccountResourceLimit(account, null, Resource.ResourceType.cpu, hostTags.get(0), 1); Mockito.doNothing().when(resourceLimitManager).checkDomainResourceLimit(account, null, Resource.ResourceType.cpu, hostTags.get(0), 1); @@ -639,7 +636,7 @@ public void testCheckResourceLimitWithTagProject() throws ResourceAllocationExce AccountVO account = Mockito.mock(AccountVO.class); Mockito.when(account.getId()).thenReturn(1L); Mockito.when(account.getType()).thenReturn(Account.Type.PROJECT); - Mockito.when(accountManager.isRootAdmin(1L)).thenReturn(false); + Mockito.when(accountManager.isRootAdmin(account)).thenReturn(false); ProjectVO projectVO = Mockito.mock(ProjectVO.class); Mockito.when(projectDao.findByProjectAccountId(Mockito.anyLong())).thenReturn(projectVO); Mockito.doReturn(new ArrayList()).when(resourceLimitManager).lockAccountAndOwnerDomainRows(Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyString()); diff --git a/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java b/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java index 6eb8bd04f46d..c3bffdd5b2a3 100644 --- a/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java +++ b/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java @@ -16,6 +16,12 @@ // under the License. package com.cloud.storage; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.doReturn; + import java.lang.reflect.Field; import java.util.ArrayList; import java.util.Arrays; @@ -24,15 +30,10 @@ import java.util.Map; import java.util.Optional; -import com.cloud.dc.HostPodVO; -import com.cloud.dc.dao.HostPodDao; -import com.cloud.host.HostVO; -import com.cloud.host.dao.HostDao; -import com.cloud.resource.ResourceManager; -import com.cloud.storage.dao.StoragePoolAndAccessGroupMapDao; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.command.admin.storage.ChangeStoragePoolScopeCmd; import org.apache.cloudstack.api.command.admin.storage.ConfigureStorageAccessCmd; +import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreLifeCycle; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; @@ -52,11 +53,14 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.object.ObjectStore; import org.apache.commons.collections.MapUtils; +import org.junit.After; import org.junit.Assert; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; @@ -72,9 +76,11 @@ import com.cloud.dc.ClusterVO; import com.cloud.dc.DataCenter; import com.cloud.dc.DataCenterVO; +import com.cloud.dc.HostPodVO; import com.cloud.dc.VsphereStoragePolicyVO; import com.cloud.dc.dao.ClusterDao; import com.cloud.dc.dao.DataCenterDao; +import com.cloud.dc.dao.HostPodDao; import com.cloud.dc.dao.VsphereStoragePolicyDao; import com.cloud.exception.AgentUnavailableException; import com.cloud.exception.ConnectionException; @@ -83,8 +89,12 @@ import com.cloud.exception.PermissionDeniedException; import com.cloud.exception.StorageUnavailableException; import com.cloud.host.Host; +import com.cloud.host.HostVO; +import com.cloud.host.dao.HostDao; import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.hypervisor.HypervisorGuruManager; +import com.cloud.resource.ResourceManager; +import com.cloud.storage.dao.StoragePoolAndAccessGroupMapDao; import com.cloud.storage.dao.VolumeDao; import com.cloud.user.AccountManagerImpl; import com.cloud.utils.Pair; @@ -93,12 +103,6 @@ import com.cloud.vm.VMInstanceVO; import com.cloud.vm.dao.VMInstanceDao; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertThrows; -import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.doReturn; - @RunWith(MockitoJUnitRunner.class) public class StorageManagerImplTest { @@ -170,6 +174,22 @@ public class StorageManagerImplTest { @Mock DataStoreManager dataStoreMgr; + @Mock + CallContext callContextMock; + + private MockedStatic callContextMockedStatic; + + @Before + public void setup() throws Exception { + callContextMockedStatic = Mockito.mockStatic(CallContext.class); + callContextMockedStatic.when(CallContext::current).thenReturn(callContextMock); + } + + @After + public void tearDown() throws Exception { + callContextMockedStatic.close(); + } + @Test public void createLocalStoragePoolName() { String hostMockName = "host1"; @@ -615,7 +635,7 @@ private void prepareTestChangeStoragePoolScope(ScopeType currentScope, StoragePo final DataCenterVO zone = new DataCenterVO(1L, null, null, null, null, null, null, null, null, null, DataCenter.NetworkType.Advanced, null, null); StoragePoolVO primaryStorage = mockStoragePoolVOForChangeStoragePoolScope(currentScope, status); - Mockito.when(accountMgr.isRootAdmin(Mockito.any())).thenReturn(true); + Mockito.when(callContextMock.isCallingAccountRootAdmin()).thenReturn(true); Mockito.when(dataCenterDao.findById(1L)).thenReturn(zone); Mockito.when(storagePoolDao.findById(1L)).thenReturn(primaryStorage); } @@ -687,9 +707,8 @@ public void testCheckNFSMountOptionsForCreateNotNFS() { public void testCheckNFSMountOptionsForUpdateNoNFSMountOptions() { Map details = new HashMap<>(); StoragePoolVO pool = new StoragePoolVO(); - Long accountId = 1L; try { - storageManagerImpl.checkNFSMountOptionsForUpdate(details, pool, accountId); + storageManagerImpl.checkNFSMountOptionsForUpdate(details, pool); } catch (Exception e) { Assert.fail(); } @@ -699,11 +718,10 @@ public void testCheckNFSMountOptionsForUpdateNoNFSMountOptions() { public void testCheckNFSMountOptionsForUpdateNotRootAdmin() { Map details = new HashMap<>(); StoragePoolVO pool = new StoragePoolVO(); - Long accountId = 1L; details.put(ApiConstants.NFS_MOUNT_OPTIONS, "vers=4.1"); - Mockito.when(accountMgr.isRootAdmin(accountId)).thenReturn(false); + Mockito.when(callContextMock.isCallingAccountRootAdmin()).thenReturn(false); PermissionDeniedException exception = assertThrows(PermissionDeniedException.class, - () -> storageManagerImpl.checkNFSMountOptionsForUpdate(details, pool, accountId)); + () -> storageManagerImpl.checkNFSMountOptionsForUpdate(details, pool)); Assert.assertEquals(exception.getMessage(), "Only root admin can modify nfs options"); } @@ -711,12 +729,11 @@ public void testCheckNFSMountOptionsForUpdateNotRootAdmin() { public void testCheckNFSMountOptionsForUpdateNotKVM() { Map details = new HashMap<>(); StoragePoolVO pool = new StoragePoolVO(); - Long accountId = 1L; details.put(ApiConstants.NFS_MOUNT_OPTIONS, "vers=4.1"); - Mockito.when(accountMgr.isRootAdmin(accountId)).thenReturn(true); pool.setHypervisor(HypervisorType.XenServer); + Mockito.when(callContextMock.isCallingAccountRootAdmin()).thenReturn(true); InvalidParameterValueException exception = assertThrows(InvalidParameterValueException.class, - () -> storageManagerImpl.checkNFSMountOptionsForUpdate(details, pool, accountId)); + () -> storageManagerImpl.checkNFSMountOptionsForUpdate(details, pool)); Assert.assertEquals(exception.getMessage(), "NFS options can only be set for the hypervisor type " + HypervisorType.KVM); } @@ -724,13 +741,12 @@ public void testCheckNFSMountOptionsForUpdateNotKVM() { public void testCheckNFSMountOptionsForUpdateNotNFS() { Map details = new HashMap<>(); StoragePoolVO pool = new StoragePoolVO(); - Long accountId = 1L; details.put(ApiConstants.NFS_MOUNT_OPTIONS, "vers=4.1"); - Mockito.when(accountMgr.isRootAdmin(accountId)).thenReturn(true); pool.setHypervisor(HypervisorType.KVM); pool.setPoolType(Storage.StoragePoolType.FiberChannel); + Mockito.when(callContextMock.isCallingAccountRootAdmin()).thenReturn(true); InvalidParameterValueException exception = assertThrows(InvalidParameterValueException.class, - () -> storageManagerImpl.checkNFSMountOptionsForUpdate(details, pool, accountId)); + () -> storageManagerImpl.checkNFSMountOptionsForUpdate(details, pool)); Assert.assertEquals(exception.getMessage(), "NFS options can only be set on pool type " + Storage.StoragePoolType.NetworkFilesystem); } @@ -738,14 +754,13 @@ public void testCheckNFSMountOptionsForUpdateNotNFS() { public void testCheckNFSMountOptionsForUpdateNotMaintenance() { Map details = new HashMap<>(); StoragePoolVO pool = new StoragePoolVO(); - Long accountId = 1L; details.put(ApiConstants.NFS_MOUNT_OPTIONS, "vers=4.1"); - Mockito.when(accountMgr.isRootAdmin(accountId)).thenReturn(true); pool.setHypervisor(HypervisorType.KVM); pool.setPoolType(Storage.StoragePoolType.NetworkFilesystem); pool.setStatus(StoragePoolStatus.Up); + Mockito.when(callContextMock.isCallingAccountRootAdmin()).thenReturn(true); InvalidParameterValueException exception = assertThrows(InvalidParameterValueException.class, - () -> storageManagerImpl.checkNFSMountOptionsForUpdate(details, pool, accountId)); + () -> storageManagerImpl.checkNFSMountOptionsForUpdate(details, pool)); Assert.assertEquals(exception.getMessage(), "The storage pool should be in maintenance mode to edit nfs options"); } @@ -766,9 +781,8 @@ public void testInvalidNFSMountOptions() { pool.setHypervisor(HypervisorType.KVM); pool.setPoolType(Storage.StoragePoolType.NetworkFilesystem); pool.setStatus(StoragePoolStatus.Maintenance); - Long accountId = 1L; - Mockito.when(accountMgr.isRootAdmin(accountId)).thenReturn(true); - storageManagerImpl.checkNFSMountOptionsForUpdate(details, pool, accountId); + Mockito.when(callContextMock.isCallingAccountRootAdmin()).thenReturn(true); + storageManagerImpl.checkNFSMountOptionsForUpdate(details, pool); } @Test diff --git a/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerImplTest.java b/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerImplTest.java index e6c2a0d0f3cf..f1ee4d3a3b70 100644 --- a/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerImplTest.java +++ b/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerImplTest.java @@ -210,7 +210,7 @@ public void testValidatePolicyZonesDisabledZone() { DataCenterVO zone1 = Mockito.mock(DataCenterVO.class); Mockito.when(zone1.getAllocationState()).thenReturn(Grouping.AllocationState.Disabled); Mockito.when(dataCenterDao.findById(2L)).thenReturn(zone1); - Mockito.when(accountManager.isRootAdmin(Mockito.any())).thenReturn(false); + Mockito.when(accountManager.isRootAdmin(Mockito.any(Account.class))).thenReturn(false); snapshotManager.validatePolicyZones(List.of(2L), volumeVO, Mockito.mock(Account.class)); } diff --git a/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java b/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java index 4ccc6e999618..147f215d2d77 100755 --- a/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java +++ b/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java @@ -16,9 +16,9 @@ // under the License. package com.cloud.storage.snapshot; -import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; @@ -34,9 +34,6 @@ import java.util.Map; import java.util.UUID; -import com.cloud.api.ApiDBUtils; -import com.cloud.exception.PermissionDeniedException; -import com.cloud.storage.Storage; import org.apache.cloudstack.api.command.user.snapshot.ExtractSnapshotCmd; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; @@ -70,11 +67,13 @@ import org.mockito.junit.MockitoJUnitRunner; import org.mockito.stubbing.Answer; +import com.cloud.api.ApiDBUtils; import com.cloud.configuration.Resource.ResourceType; import com.cloud.dc.DataCenter; import com.cloud.dc.DataCenterVO; import com.cloud.dc.dao.DataCenterDao; import com.cloud.exception.InvalidParameterValueException; +import com.cloud.exception.PermissionDeniedException; import com.cloud.exception.ResourceAllocationException; import com.cloud.hypervisor.Hypervisor; import com.cloud.hypervisor.Hypervisor.HypervisorType; @@ -86,6 +85,7 @@ import com.cloud.storage.Snapshot; import com.cloud.storage.SnapshotPolicyVO; import com.cloud.storage.SnapshotVO; +import com.cloud.storage.Storage; import com.cloud.storage.Volume; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.SnapshotDao; @@ -561,7 +561,7 @@ public void testIsBackupSnapshotToSecondaryForEdgeZone() { private void mockForExtractSnapshotTests() { Mockito.doReturn(TEST_SNAPSHOT_ID).when(extractSnapshotCmdMock).getId(); Mockito.doReturn(TEST_ZONE_ID).when(extractSnapshotCmdMock).getZoneId(); - Mockito.doReturn(false).when(_accountMgr).isRootAdmin(Mockito.anyLong()); + Mockito.doReturn(false).when(_accountMgr).isRootAdmin(any(Account.class)); Mockito.when(ApiDBUtils.isExtractionDisabled()).thenReturn(false); Mockito.doReturn(dataCenterVOMock).when(dataCenterDao).findById(TEST_ZONE_ID); @@ -654,7 +654,7 @@ public void extractSnapshotTestCreateExtractUrlReturnUrl() { @Test() public void extractSnapshotTestRootAdminDisabledExtractionCreateExtractUrlReturnUrl() { mockForExtractSnapshotTests(); - Mockito.doReturn(true).when(_accountMgr).isRootAdmin(Mockito.anyLong()); + Mockito.doReturn(true).when(_accountMgr).isRootAdmin(any(Account.class)); Mockito.when(ApiDBUtils.isExtractionDisabled()).thenReturn(true); Assert.assertEquals(TEST_EXTRACT_URL, _snapshotMgr.extractSnapshot(extractSnapshotCmdMock)); diff --git a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java index 3055e48247c4..49243ce9f6d0 100644 --- a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java +++ b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java @@ -381,42 +381,20 @@ public void testGetUserKeysCmdDomainAdminRootAdminUser() { accountManagerImpl.getKeys(_listkeyscmd); } - @Test - public void testPreventRootDomainAdminAccessToRootAdminKeysNormalUser() { - User user = Mockito.mock(User.class); - ControlledEntity entity = Mockito.mock(ControlledEntity.class); - Mockito.when(user.getAccountId()).thenReturn(1L); - AccountVO account = Mockito.mock(AccountVO.class); - Mockito.when(account.getType()).thenReturn(Account.Type.NORMAL); - Mockito.when(_accountDao.findById(1L)).thenReturn(account); - accountManagerImpl.preventRootDomainAdminAccessToRootAdminKeys(user, entity); - Mockito.verify(accountManagerImpl, Mockito.never()).isRootAdmin(Mockito.anyLong()); - } - - @Test(expected = PermissionDeniedException.class) - public void testPreventRootDomainAdminAccessToRootAdminKeysRootDomainAdminUser() { - User user = Mockito.mock(User.class); - ControlledEntity entity = Mockito.mock(ControlledEntity.class); - Mockito.when(user.getAccountId()).thenReturn(1L); - AccountVO account = Mockito.mock(AccountVO.class); - Mockito.when(account.getType()).thenReturn(Account.Type.DOMAIN_ADMIN); - Mockito.when(account.getDomainId()).thenReturn(Domain.ROOT_DOMAIN); - Mockito.when(_accountDao.findById(1L)).thenReturn(account); - Mockito.when(entity.getAccountId()).thenReturn(1L); - Mockito.lenient().when(securityChecker.checkAccess(Mockito.any(Account.class), - Mockito.nullable(ControlledEntity.class), Mockito.nullable(AccessType.class), Mockito.anyString())).thenReturn(true); - accountManagerImpl.preventRootDomainAdminAccessToRootAdminKeys(user, entity); - } - @Test public void updateUserTestTimeZoneAndEmailNull() { Mockito.when(userVoMock.getAccountId()).thenReturn(10L); Mockito.doReturn(accountMock).when(accountManagerImpl).getAccount(10L); - Mockito.when(accountMock.getAccountId()).thenReturn(10L); - Mockito.doReturn(false).when(accountManagerImpl).isRootAdmin(10L); + Mockito.doReturn(false).when(accountManagerImpl).isRootAdmin(accountMock); Mockito.lenient().when(accountManagerImpl.getRoleType(Mockito.eq(accountMock))).thenReturn(RoleType.User); - prepareMockAndExecuteUpdateUserTest(0); + try (MockedStatic callContextMockedStatic = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMockedStatic.when(CallContext::current).thenReturn(callContextMock); + Mockito.doReturn(false).when(callContextMock).isCallingAccountRootAdmin(); + Mockito.doReturn(userVoMock).when(callContextMock).getCallingUser(); + prepareMockAndExecuteUpdateUserTest(0); + } } @Test @@ -425,10 +403,16 @@ public void updateUserTestTimeZoneAndEmailNotNull() { Mockito.when(UpdateUserCmdMock.getTimezone()).thenReturn("timezone"); Mockito.when(userVoMock.getAccountId()).thenReturn(10L); Mockito.doReturn(accountMock).when(accountManagerImpl).getAccount(10L); - Mockito.when(accountMock.getAccountId()).thenReturn(10L); - Mockito.doReturn(false).when(accountManagerImpl).isRootAdmin(10L); + Mockito.doReturn(false).when(accountManagerImpl).isRootAdmin(accountMock); Mockito.lenient().when(accountManagerImpl.getRoleType(Mockito.eq(accountMock))).thenReturn(RoleType.User); - prepareMockAndExecuteUpdateUserTest(1); + + try (MockedStatic callContextMockedStatic = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMockedStatic.when(CallContext::current).thenReturn(callContextMock); + Mockito.doReturn(false).when(callContextMock).isCallingAccountRootAdmin(); + Mockito.doReturn(userVoMock).when(callContextMock).getCallingUser(); + prepareMockAndExecuteUpdateUserTest(1); + } } private void prepareMockAndExecuteUpdateUserTest(int numberOfExpectedCallsForSetEmailAndSetTimeZone) { @@ -798,21 +782,23 @@ public void valiateUserPasswordAndUpdateIfNeededTestBlankPassword() { @Test(expected = InvalidParameterValueException.class) public void valiateUserPasswordAndUpdateIfNeededTestNoAdminAndNoCurrentPasswordProvided() { Mockito.doReturn(accountMock).when(accountManagerImpl).getCurrentCallingAccount(); - Mockito.doReturn(false).when(accountManagerImpl).isRootAdmin(accountMockId); Mockito.doReturn(false).when(accountManagerImpl).isDomainAdmin(accountMockId); Mockito.lenient().doReturn(true).when(accountManagerImpl).isResourceDomainAdmin(accountMockId); Mockito.doReturn(accountMock).when(accountManagerImpl).getAccount(Mockito.anyLong()); Mockito.lenient().doNothing().when(passwordPolicyMock).verifyIfPasswordCompliesWithPasswordPolicies(Mockito.anyString(), Mockito.anyString(), Mockito.anyLong()); - - accountManagerImpl.validateUserPasswordAndUpdateIfNeeded("newPassword", userVoMock, " ", false); + try (MockedStatic callContextMockedStatic = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMockedStatic.when(CallContext::current).thenReturn(callContextMock); + Mockito.doReturn(false).when(callContextMock).isCallingAccountRootAdmin(); + accountManagerImpl.validateUserPasswordAndUpdateIfNeeded("newPassword", userVoMock, " ", false); + } } @Test(expected = CloudRuntimeException.class) public void valiateUserPasswordAndUpdateIfNeededTestNoUserAuthenticatorsConfigured() { Mockito.doReturn(accountMock).when(accountManagerImpl).getCurrentCallingAccount(); - Mockito.doReturn(true).when(accountManagerImpl).isRootAdmin(accountMockId); Mockito.doReturn(false).when(accountManagerImpl).isDomainAdmin(accountMockId); Mockito.lenient().doNothing().when(accountManagerImpl).validateCurrentPassword(Mockito.eq(userVoMock), Mockito.anyString()); @@ -821,13 +807,17 @@ public void valiateUserPasswordAndUpdateIfNeededTestNoUserAuthenticatorsConfigur Mockito.lenient().doNothing().when(passwordPolicyMock).verifyIfPasswordCompliesWithPasswordPolicies(Mockito.anyString(), Mockito.anyString(), Mockito.anyLong()); - accountManagerImpl.validateUserPasswordAndUpdateIfNeeded("newPassword", userVoMock, null, false); + try (MockedStatic callContextMockedStatic = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMockedStatic.when(CallContext::current).thenReturn(callContextMock); + Mockito.doReturn(true).when(callContextMock).isCallingAccountRootAdmin(); + accountManagerImpl.validateUserPasswordAndUpdateIfNeeded("newPassword", userVoMock, null, false); + } } @Test public void validateUserPasswordAndUpdateIfNeededTestRootAdminUpdatingUserPassword() { Mockito.doReturn(accountMock).when(accountManagerImpl).getCurrentCallingAccount(); - Mockito.doReturn(true).when(accountManagerImpl).isRootAdmin(accountMockId); Mockito.doReturn(false).when(accountManagerImpl).isDomainAdmin(accountMockId); String newPassword = "newPassword"; @@ -840,16 +830,20 @@ public void validateUserPasswordAndUpdateIfNeededTestRootAdminUpdatingUserPasswo Mockito.lenient().doNothing().when(passwordPolicyMock).verifyIfPasswordCompliesWithPasswordPolicies(Mockito.anyString(), Mockito.anyString(), Mockito.anyLong()); - accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(newPassword, userVoMock, null, false); + try (MockedStatic callContextMockedStatic = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMockedStatic.when(CallContext::current).thenReturn(callContextMock); + Mockito.doReturn(true).when(callContextMock).isCallingAccountRootAdmin(); + accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(newPassword, userVoMock, null, false); - Mockito.verify(accountManagerImpl, Mockito.times(0)).validateCurrentPassword(Mockito.eq(userVoMock), Mockito.anyString()); - Mockito.verify(userVoMock, Mockito.times(1)).setPassword(expectedUserPasswordAfterEncoded); + Mockito.verify(accountManagerImpl, Mockito.times(0)).validateCurrentPassword(Mockito.eq(userVoMock), Mockito.anyString()); + Mockito.verify(userVoMock, Mockito.times(1)).setPassword(expectedUserPasswordAfterEncoded); + } } @Test public void validateUserPasswordAndUpdateIfNeededTestDomainAdminUpdatingUserPassword() { Mockito.doReturn(accountMock).when(accountManagerImpl).getCurrentCallingAccount(); - Mockito.doReturn(false).when(accountManagerImpl).isRootAdmin(accountMockId); Mockito.doReturn(true).when(accountManagerImpl).isDomainAdmin(accountMockId); String newPassword = "newPassword"; @@ -861,17 +855,21 @@ public void validateUserPasswordAndUpdateIfNeededTestDomainAdminUpdatingUserPass Mockito.doReturn(accountMock).when(accountManagerImpl).getAccount(Mockito.anyLong()); Mockito.lenient().doNothing().when(passwordPolicyMock).verifyIfPasswordCompliesWithPasswordPolicies(Mockito.anyString(), Mockito.anyString(), Mockito.anyLong()); + try (MockedStatic callContextMockedStatic = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMockedStatic.when(CallContext::current).thenReturn(callContextMock); + Mockito.doReturn(false).when(callContextMock).isCallingAccountRootAdmin(); - accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(newPassword, userVoMock, null, false); + accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(newPassword, userVoMock, null, false); - Mockito.verify(accountManagerImpl, Mockito.times(0)).validateCurrentPassword(Mockito.eq(userVoMock), Mockito.anyString()); - Mockito.verify(userVoMock, Mockito.times(1)).setPassword(expectedUserPasswordAfterEncoded); + Mockito.verify(accountManagerImpl, Mockito.times(0)).validateCurrentPassword(Mockito.eq(userVoMock), Mockito.anyString()); + Mockito.verify(userVoMock, Mockito.times(1)).setPassword(expectedUserPasswordAfterEncoded); + } } @Test public void validateUserPasswordAndUpdateIfNeededTestUserUpdatingHisPassword() { Mockito.doReturn(accountMock).when(accountManagerImpl).getCurrentCallingAccount(); - Mockito.doReturn(false).when(accountManagerImpl).isRootAdmin(accountMockId); Mockito.doReturn(false).when(accountManagerImpl).isDomainAdmin(accountMockId); String newPassword = "newPassword"; @@ -884,11 +882,16 @@ public void validateUserPasswordAndUpdateIfNeededTestUserUpdatingHisPassword() { Mockito.doReturn(accountMock).when(accountManagerImpl).getAccount(Mockito.anyLong()); Mockito.lenient().doNothing().when(passwordPolicyMock).verifyIfPasswordCompliesWithPasswordPolicies(Mockito.anyString(), Mockito.anyString(), Mockito.anyLong()); + try (MockedStatic callContextMockedStatic = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMockedStatic.when(CallContext::current).thenReturn(callContextMock); + Mockito.doReturn(false).when(callContextMock).isCallingAccountRootAdmin(); - accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(newPassword, userVoMock, currentPassword, false); + accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(newPassword, userVoMock, currentPassword, false); - Mockito.verify(accountManagerImpl, Mockito.times(1)).validateCurrentPassword(userVoMock, currentPassword); - Mockito.verify(userVoMock, Mockito.times(1)).setPassword(expectedUserPasswordAfterEncoded); + Mockito.verify(accountManagerImpl, Mockito.times(1)).validateCurrentPassword(userVoMock, currentPassword); + Mockito.verify(userVoMock, Mockito.times(1)).setPassword(expectedUserPasswordAfterEncoded); + } } @Test (expected = InvalidParameterValueException.class) @@ -1529,8 +1532,12 @@ public void testCheckCallerRoleTypeAllowedToUpdateUserLowerAccountRoleType() { public void testcheckCallerApiPermissionsForUserOperationsRootAdminSameCaller() { Mockito.lenient().when(accountManagerImpl.getCurrentCallingAccount()).thenReturn(accountMock); Mockito.when(accountMock.getId()).thenReturn(2L); - Mockito.doReturn(true).when(accountManagerImpl).isRootAdmin(2L); - accountManagerImpl.checkCallerApiPermissionsForUserOrAccountOperations(accountMock); + try (MockedStatic callContextMockedStatic = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMockedStatic.when(CallContext::current).thenReturn(callContextMock); + Mockito.doReturn(true).when(callContextMock).isCallingAccountRootAdmin(); + accountManagerImpl.checkCallerApiPermissionsForUserOrAccountOperations(accountMock); + } } @Test(expected = PermissionDeniedException.class) @@ -1538,12 +1545,15 @@ public void testcheckCallerApiPermissionsForUserOperationsRootAdminDifferentAcco Mockito.lenient().when(accountManagerImpl.getCurrentCallingAccount()).thenReturn(callingAccount); Mockito.lenient().when(callingAccount.getAccountId()).thenReturn(3L); Mockito.lenient().doReturn(callingAccount).when(accountManagerImpl).getAccount(3L); - Mockito.lenient().doReturn(false).when(accountManagerImpl).isRootAdmin(3L); - Mockito.when(accountMock.getAccountId()).thenReturn(2L); - Mockito.doReturn(true).when(accountManagerImpl).isRootAdmin(2L); + Mockito.doReturn(true).when(accountManagerImpl).isRootAdmin(accountMock); - accountManagerImpl.checkCallerApiPermissionsForUserOrAccountOperations(accountMock); + try (MockedStatic callContextMockedStatic = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMockedStatic.when(CallContext::current).thenReturn(callContextMock); + Mockito.doReturn(false).when(callContextMock).isCallingAccountRootAdmin(); + accountManagerImpl.checkCallerApiPermissionsForUserOrAccountOperations(accountMock); + } } @Test @@ -1551,14 +1561,17 @@ public void testcheckCallerApiPermissionsForUserOperationsAllowedApis() { Mockito.lenient().when(accountManagerImpl.getCurrentCallingAccount()).thenReturn(callingAccount); Mockito.lenient().when(callingAccount.getAccountId()).thenReturn(3L); Mockito.lenient().doReturn(callingAccount).when(accountManagerImpl).getAccount(3L); - Mockito.lenient().doReturn(false).when(accountManagerImpl).isRootAdmin(3L); - Mockito.when(accountMock.getAccountId()).thenReturn(2L); - Mockito.doReturn(false).when(accountManagerImpl).isRootAdmin(2L); + Mockito.doReturn(false).when(accountManagerImpl).isRootAdmin(accountMock); Mockito.lenient().doNothing().when(accountManagerImpl).checkRoleEscalation(callingAccount, accountMock); - accountManagerImpl.checkCallerApiPermissionsForUserOrAccountOperations(accountMock); + try (MockedStatic callContextMockedStatic = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMockedStatic.when(CallContext::current).thenReturn(callContextMock); + Mockito.doReturn(false).when(callContextMock).isCallingAccountRootAdmin(); + accountManagerImpl.checkCallerApiPermissionsForUserOrAccountOperations(accountMock); + } } @Test(expected = PermissionDeniedException.class) @@ -1566,13 +1579,16 @@ public void testcheckCallerApiPermissionsForUserOperationsNotAllowedApis() { Mockito.lenient().when(accountManagerImpl.getCurrentCallingAccount()).thenReturn(callingAccount); Mockito.lenient().when(callingAccount.getAccountId()).thenReturn(3L); Mockito.lenient().doReturn(callingAccount).when(accountManagerImpl).getAccount(3L); - Mockito.lenient().doReturn(false).when(accountManagerImpl).isRootAdmin(3L); - Mockito.when(accountMock.getAccountId()).thenReturn(2L); - Mockito.doReturn(false).when(accountManagerImpl).isRootAdmin(2L); + Mockito.doReturn(false).when(accountManagerImpl).isRootAdmin(accountMock); Mockito.lenient().doThrow(PermissionDeniedException.class).when(accountManagerImpl).checkRoleEscalation(callingAccount, accountMock); - accountManagerImpl.checkCallerApiPermissionsForUserOrAccountOperations(accountMock); + try (MockedStatic callContextMockedStatic = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMockedStatic.when(CallContext::current).thenReturn(callContextMock); + Mockito.doReturn(false).when(callContextMock).isCallingAccountRootAdmin(); + accountManagerImpl.checkCallerApiPermissionsForUserOrAccountOperations(accountMock); + } } } diff --git a/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java b/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java index a84f02755c7c..32641e218d1e 100644 --- a/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java +++ b/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java @@ -213,6 +213,12 @@ public boolean isRootAdmin(Long accountId) { return false; } + @Override + public boolean isRootAdmin(Account account) { + // TODO Auto-generated method stub + return false; + } + @Override public User getActiveUserByRegistrationToken(String registrationToken) { // TODO Auto-generated method stub diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index e1efd87dcd8b..2e417861b915 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -135,9 +135,9 @@ import com.cloud.network.dao.LoadBalancerVMMapVO; import com.cloud.network.dao.NetworkDao; import com.cloud.network.dao.NetworkVO; -import com.cloud.network.element.UserDataServiceProvider; import com.cloud.network.dao.PhysicalNetworkDao; import com.cloud.network.dao.PhysicalNetworkVO; +import com.cloud.network.element.UserDataServiceProvider; import com.cloud.network.guru.NetworkGuru; import com.cloud.network.rules.FirewallRuleVO; import com.cloud.network.rules.PortForwardingRule; @@ -431,6 +431,11 @@ public class UserVmManagerImplTest { @Mock private UUIDManager uuidMgr; + @Mock + CallContext callContextMock; + + private MockedStatic callContextMockedStatic; + private static final long vmId = 1l; private static final long zoneId = 2L; private static final long accountId = 3L; @@ -461,7 +466,9 @@ public void beforeTest() { Mockito.when(userVmDao.findById(vmId)).thenReturn(userVmVoMock); Mockito.when(callerAccount.getType()).thenReturn(Account.Type.ADMIN); - CallContext.register(callerUser, callerAccount); + callContextMockedStatic = Mockito.mockStatic(CallContext.class); + callContextMockedStatic.when(CallContext::current).thenReturn(callContextMock); + Mockito.when(callContextMock.getCallingAccount()).thenReturn(callerAccount); customParameters.put(VmDetailConstants.ROOT_DISK_SIZE, "123"); customParameters.put(VmDetailConstants.MEMORY, "2048"); @@ -476,7 +483,7 @@ public void beforeTest() { @After public void afterTest() { - CallContext.unregister(); + callContextMockedStatic.close(); } @Test @@ -2613,113 +2620,81 @@ public void canAccountUseNetworkTestPermissionDeniedExceptionThrownReturnFalse() @Test public void implementNetworkTestImplementedNetworkIsNullReturnCurrentNewNetwork() throws ResourceUnavailableException, InsufficientCapacityException { - CallContext callContextMock = Mockito.mock(CallContext.class); NetworkVO currentNetwork = Mockito.mock(NetworkVO.class); + Mockito.doReturn(1l).when(callContextMock).getCallingUserId(); - try (MockedStatic ignored = Mockito.mockStatic(CallContext.class)) { - Mockito.when(CallContext.current()).thenReturn(callContextMock); - - Mockito.doReturn(1l).when(callContextMock).getCallingUserId(); - - Mockito.doReturn(callerUser).when(userDao).findById(Mockito.anyLong()); - Mockito.doReturn(null).when(_networkMgr).implementNetwork(Mockito.anyLong(), Mockito.any(), Mockito.any()); + Mockito.doReturn(callerUser).when(userDao).findById(Mockito.anyLong()); + Mockito.doReturn(null).when(_networkMgr).implementNetwork(Mockito.anyLong(), Mockito.any(), Mockito.any()); - Network newNetwork = userVmManagerImpl.implementNetwork(accountMock, _dcMock, currentNetwork); + Network newNetwork = userVmManagerImpl.implementNetwork(accountMock, _dcMock, currentNetwork); - Assert.assertEquals(newNetwork, currentNetwork); - } + Assert.assertEquals(newNetwork, currentNetwork); } @Test public void implementNetworkTestImplementedNetworkFirstIsNullReturnCurrentNewNetwork() throws ResourceUnavailableException, InsufficientCapacityException { - CallContext callContextMock = Mockito.mock(CallContext.class); NetworkVO currentNetwork = Mockito.mock(NetworkVO.class); + Mockito.doReturn(1l).when(callContextMock).getCallingUserId(); - try (MockedStatic ignored = Mockito.mockStatic(CallContext.class)) { - Mockito.when(CallContext.current()).thenReturn(callContextMock); + Pair implementedNetwork = Mockito.mock(Pair.class); - Mockito.doReturn(1l).when(callContextMock).getCallingUserId(); + Mockito.doReturn(callerUser).when(userDao).findById(Mockito.anyLong()); + Mockito.doReturn(null).when(implementedNetwork).first(); + Mockito.doReturn(implementedNetwork).when(_networkMgr).implementNetwork(Mockito.anyLong(), Mockito.any(), Mockito.any()); - Pair implementedNetwork = Mockito.mock(Pair.class); + Network newNetwork = userVmManagerImpl.implementNetwork(accountMock, _dcMock, currentNetwork); - Mockito.doReturn(callerUser).when(userDao).findById(Mockito.anyLong()); - Mockito.doReturn(null).when(implementedNetwork).first(); - Mockito.doReturn(implementedNetwork).when(_networkMgr).implementNetwork(Mockito.anyLong(), Mockito.any(), Mockito.any()); - - Network newNetwork = userVmManagerImpl.implementNetwork(accountMock, _dcMock, currentNetwork); - - Assert.assertEquals(newNetwork, currentNetwork); - } + Assert.assertEquals(newNetwork, currentNetwork); } @Test public void implementNetworkTestImplementedNetworkSecondIsNullReturnCurrentNewNetwork() throws ResourceUnavailableException, InsufficientCapacityException { - CallContext callContextMock = Mockito.mock(CallContext.class); NetworkVO currentNetwork = Mockito.mock(NetworkVO.class); + Mockito.doReturn(1l).when(callContextMock).getCallingUserId(); - try (MockedStatic ignored = Mockito.mockStatic(CallContext.class)) { - Mockito.when(CallContext.current()).thenReturn(callContextMock); - - Mockito.doReturn(1l).when(callContextMock).getCallingUserId(); + Pair implementedNetwork = Mockito.mock(Pair.class); - Pair implementedNetwork = Mockito.mock(Pair.class); + Mockito.doReturn(callerUser).when(userDao).findById(Mockito.anyLong()); + Mockito.doReturn(networkMock).when(implementedNetwork).first(); + Mockito.doReturn(null).when(implementedNetwork).second(); + Mockito.doReturn(implementedNetwork).when(_networkMgr).implementNetwork(Mockito.anyLong(), Mockito.any(), Mockito.any()); - Mockito.doReturn(callerUser).when(userDao).findById(Mockito.anyLong()); - Mockito.doReturn(networkMock).when(implementedNetwork).first(); - Mockito.doReturn(null).when(implementedNetwork).second(); - Mockito.doReturn(implementedNetwork).when(_networkMgr).implementNetwork(Mockito.anyLong(), Mockito.any(), Mockito.any()); + Network newNetwork = userVmManagerImpl.implementNetwork(accountMock, _dcMock, currentNetwork); - Network newNetwork = userVmManagerImpl.implementNetwork(accountMock, _dcMock, currentNetwork); - - Assert.assertEquals(newNetwork, currentNetwork); - } + Assert.assertEquals(newNetwork, currentNetwork); } @Test public void implementNetworkTestImplementedNetworkSecondIsNotNullReturnImplementedNetworkSecond() throws ResourceUnavailableException, InsufficientCapacityException { - CallContext callContextMock = Mockito.mock(CallContext.class); NetworkVO currentNetwork = Mockito.mock(NetworkVO.class); + Mockito.when(CallContext.current()).thenReturn(callContextMock); - try (MockedStatic ignored = Mockito.mockStatic(CallContext.class)) { - Mockito.when(CallContext.current()).thenReturn(callContextMock); - - Mockito.doReturn(1l).when(callContextMock).getCallingUserId(); + Mockito.doReturn(1l).when(callContextMock).getCallingUserId(); - Pair implementedNetwork = Mockito.mock(Pair.class); + Pair implementedNetwork = Mockito.mock(Pair.class); - Mockito.doReturn(callerUser).when(userDao).findById(Mockito.anyLong()); - Mockito.doReturn(networkMock).when(implementedNetwork).first(); - Mockito.doReturn(networkMock).when(implementedNetwork).second(); - Mockito.doReturn(implementedNetwork).when(_networkMgr).implementNetwork(Mockito.anyLong(), Mockito.any(), Mockito.any()); + Mockito.doReturn(callerUser).when(userDao).findById(Mockito.anyLong()); + Mockito.doReturn(networkMock).when(implementedNetwork).first(); + Mockito.doReturn(networkMock).when(implementedNetwork).second(); + Mockito.doReturn(implementedNetwork).when(_networkMgr).implementNetwork(Mockito.anyLong(), Mockito.any(), Mockito.any()); - Network newNetwork = userVmManagerImpl.implementNetwork(accountMock, _dcMock, currentNetwork); + Network newNetwork = userVmManagerImpl.implementNetwork(accountMock, _dcMock, currentNetwork); - Assert.assertEquals(newNetwork, networkMock); - } + Assert.assertEquals(newNetwork, networkMock); } @Test public void implementNetworkTestImplementedNetworkCatchException() throws ResourceUnavailableException, InsufficientCapacityException { String expectedMessage = String.format("Failed to implement network [%s] elements and resources as a part of network provision.", networkMock); + Mockito.doReturn(1l).when(callContextMock).getCallingUserId(); + Mockito.doReturn(callerUser).when(userDao).findById(Mockito.anyLong()); + Mockito.doThrow(InvalidParameterValueException.class).when(_networkMgr).implementNetwork(Mockito.anyLong(), Mockito.any(), Mockito.any()); - CallContext callContextMock = Mockito.mock(CallContext.class); - - try (MockedStatic ignored = Mockito.mockStatic(CallContext.class)) { - Mockito.when(CallContext.current()).thenReturn(callContextMock); - - Mockito.doReturn(1l).when(callContextMock).getCallingUserId(); - - Pair implementedNetwork = Mockito.mock(Pair.class); - - Mockito.doReturn(callerUser).when(userDao).findById(Mockito.anyLong()); - Mockito.doThrow(InvalidParameterValueException.class).when(_networkMgr).implementNetwork(Mockito.anyLong(), Mockito.any(), Mockito.any()); - - CloudRuntimeException assertThrows = Assert.assertThrows(expectedCloudRuntimeException, () -> { - userVmManagerImpl.implementNetwork(accountMock, _dcMock, networkMock); - }); + CloudRuntimeException assertThrows = Assert.assertThrows(expectedCloudRuntimeException, () -> { + userVmManagerImpl.implementNetwork(accountMock, _dcMock, networkMock); + }); - Assert.assertEquals(expectedMessage, assertThrows.getMessage()); - } + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); } @Test @@ -2954,7 +2929,7 @@ public void validateIfVolumesHaveNoSnapshotsTestVolumeHasNoSnapshotsDoesNotThrow public void moveVmToUserTestCallerIsNotRootAdminAndDomainAdminThrowsInvalidParameterValueException() { String expectedMessage = String.format("Only root or domain admins are allowed to assign VMs. Caller [%s] is of type [%s].", callerAccount, callerAccount.getType()); - Mockito.doReturn(false).when(accountManager).isRootAdmin(Mockito.anyLong()); + Mockito.doReturn(false).when(callContextMock).isCallingAccountRootAdmin(); Mockito.doReturn(false).when(accountManager).isDomainAdmin(Mockito.anyLong()); InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { @@ -2966,7 +2941,7 @@ public void moveVmToUserTestCallerIsNotRootAdminAndDomainAdminThrowsInvalidParam @Test public void moveVmToUserTestValidateVmExistsAndIsNotRunningThrowsInvalidParameterValueException() { - Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong()); + Mockito.doReturn(true).when(callContextMock).isCallingAccountRootAdmin(); Mockito.doThrow(InvalidParameterValueException.class).when(userVmManagerImpl).validateIfVmSupportsMigration(Mockito.any(), Mockito.anyLong()); @@ -2975,7 +2950,7 @@ public void moveVmToUserTestValidateVmExistsAndIsNotRunningThrowsInvalidParamete @Test public void moveVmToUserTestValidateAccountsAndCallerAccessToThemThrowsInvalidParameterValueException() { - Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong()); + Mockito.doReturn(true).when(callContextMock).isCallingAccountRootAdmin(); Mockito.doReturn(userVmVoMock).when(userVmDao).findById(Mockito.anyLong()); Assert.assertThrows(InvalidParameterValueException.class, () -> userVmManagerImpl.moveVmToUser(assignVmCmdMock)); @@ -2987,7 +2962,7 @@ public void moveVmToUserTestProjectIdProvidedAndDomainIdIsNullThrowsInvalidParam String expectedMessage = "Please provide a valid domain ID; cannot assign VM to a project if domain ID is NULL."; - Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong()); + Mockito.doReturn(true).when(callContextMock).isCallingAccountRootAdmin(); Mockito.doReturn(userVmVoMock).when(userVmDao).findById(Mockito.anyLong()); Mockito.doReturn(1l).when(assignVmCmdMock).getProjectId(); Mockito.doReturn(null).when(assignVmCmdMock).getDomainId(); @@ -3005,7 +2980,7 @@ public void moveVmToUserTestProjectIdProvidedAndDomainIdIsNullThrowsInvalidParam public void moveVmToUserTestValidateIfVmHasNoRulesThrowsInvalidParameterValueException() throws ResourceUnavailableException, InsufficientCapacityException, ResourceAllocationException { - Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong()); + Mockito.doReturn(true).when(callContextMock).isCallingAccountRootAdmin(); Mockito.doReturn(userVmVoMock).when(userVmDao).findById(Mockito.anyLong()); Mockito.doReturn(null).when(assignVmCmdMock).getProjectId(); @@ -3023,7 +2998,7 @@ public void moveVmToUserTestSnapshotsForVolumeExistThrowsInvalidParameterValueEx LinkedList volumes = new LinkedList(); volumes.add(volumeVOMock); - Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong()); + Mockito.doReturn(true).when(callContextMock).isCallingAccountRootAdmin(); Mockito.doReturn(userVmVoMock).when(userVmDao).findById(Mockito.anyLong()); Mockito.doReturn(null).when(assignVmCmdMock).getProjectId(); Mockito.doReturn(volumes).when(volumeDaoMock).findByInstance(Mockito.anyLong()); @@ -3041,7 +3016,7 @@ public void moveVmToUserTestVerifyResourceLimitsForAccountAndStorageThrowsResour LinkedList volumes = new LinkedList(); - Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong()); + Mockito.doReturn(true).when(callContextMock).isCallingAccountRootAdmin(); Mockito.doReturn(userVmVoMock).when(userVmDao).findById(Mockito.anyLong()); Mockito.doReturn(null).when(assignVmCmdMock).getProjectId(); Mockito.doReturn(volumes).when(volumeDaoMock).findByInstance(Mockito.anyLong()); @@ -3060,7 +3035,7 @@ public void moveVmToUserTestVerifyValidateIfNewOwnerHasAccessToTemplateThrowsInv LinkedList volumes = new LinkedList(); - Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong()); + Mockito.doReturn(true).when(callContextMock).isCallingAccountRootAdmin(); Mockito.doReturn(userVmVoMock).when(userVmDao).findById(Mockito.anyLong()); Mockito.doReturn(null).when(assignVmCmdMock).getProjectId(); Mockito.doReturn(volumes).when(volumeDaoMock).findByInstance(Mockito.anyLong()); @@ -3078,7 +3053,7 @@ public void moveVmToUserTestAccountManagerCheckAccessThrowsPermissionDeniedExcep LinkedList volumes = new LinkedList(); - Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong()); + Mockito.doReturn(true).when(callContextMock).isCallingAccountRootAdmin(); Mockito.doReturn(userVmVoMock).when(userVmDao).findById(Mockito.anyLong()); Mockito.doReturn(null).when(assignVmCmdMock).getProjectId(); Mockito.doReturn(volumes).when(volumeDaoMock).findByInstance(Mockito.anyLong()); @@ -3612,53 +3587,47 @@ public void testDestroyVm() throws ResourceUnavailableException { boolean expunge = true; ReflectionTestUtils.setField(userVmManagerImpl, "_uuidMgr", uuidMgr); - CallContext callContext = mock(CallContext.class); - Account callingAccount = mock(Account.class); - when(callingAccount.getId()).thenReturn(accountId); - when(callContext.getCallingAccount()).thenReturn(callingAccount); - when(accountManager.isAdmin(callingAccount.getId())).thenReturn(true); - doNothing().when(accountManager).checkApiAccess(callingAccount, BaseCmd.getCommandNameByClass(ExpungeVMCmd.class)); - try (MockedStatic mockedCallContext = Mockito.mockStatic(CallContext.class)) { - mockedCallContext.when(CallContext::current).thenReturn(callContext); - mockedCallContext.when(() -> CallContext.register(callContext, ApiCommandResourceType.Volume)).thenReturn(callContext); - - DestroyVMCmd cmd = mock(DestroyVMCmd.class); - when(cmd.getId()).thenReturn(vmId); - when(cmd.getExpunge()).thenReturn(expunge); - List volumeIds = List.of(volumeId); - when(cmd.getVolumeIds()).thenReturn(volumeIds); - - UserVmVO vm = mock(UserVmVO.class); - when(vm.getId()).thenReturn(vmId); - when(vm.getState()).thenReturn(VirtualMachine.State.Running); - when(vm.getUuid()).thenReturn("vm-uuid"); - when(vm.getUserVmType()).thenReturn("User"); - when(userVmDao.findById(vmId)).thenReturn(vm); + when(callerAccount.getId()).thenReturn(accountId); + when(accountManager.isAdmin(callerAccount.getId())).thenReturn(true); + doNothing().when(accountManager).checkApiAccess(callerAccount, BaseCmd.getCommandNameByClass(ExpungeVMCmd.class)); + callContextMockedStatic.when(() -> CallContext.register(callContextMock, ApiCommandResourceType.Volume)).thenReturn(callContextMock); - VolumeVO vol = Mockito.mock(VolumeVO.class); - when(vol.getInstanceId()).thenReturn(vmId); - when(vol.getId()).thenReturn(volumeId); - when(vol.getVolumeType()).thenReturn(Volume.Type.DATADISK); - when(volumeDaoMock.findById(volumeId)).thenReturn(vol); + DestroyVMCmd cmd = mock(DestroyVMCmd.class); + when(cmd.getId()).thenReturn(vmId); + when(cmd.getExpunge()).thenReturn(expunge); + List volumeIds = List.of(volumeId); + when(cmd.getVolumeIds()).thenReturn(volumeIds); - List dataVolumes = new ArrayList<>(); - when(volumeDaoMock.findByInstanceAndType(vmId, Volume.Type.DATADISK)).thenReturn(dataVolumes); + UserVmVO vm = mock(UserVmVO.class); + when(vm.getId()).thenReturn(vmId); + when(vm.getState()).thenReturn(VirtualMachine.State.Running); + when(vm.getUuid()).thenReturn("vm-uuid"); + when(vm.getUserVmType()).thenReturn("User"); + when(userVmDao.findById(vmId)).thenReturn(vm); - when(volumeApiService.destroyVolume(volumeId, CallContext.current().getCallingAccount(), expunge, false)).thenReturn(vol); + VolumeVO vol = Mockito.mock(VolumeVO.class); + when(vol.getInstanceId()).thenReturn(vmId); + when(vol.getId()).thenReturn(volumeId); + when(vol.getVolumeType()).thenReturn(Volume.Type.DATADISK); + when(volumeDaoMock.findById(volumeId)).thenReturn(vol); - doReturn(vm).when(userVmManagerImpl).stopVirtualMachine(anyLong(), anyBoolean()); - doReturn(vm).when(userVmManagerImpl).destroyVm(vmId, expunge); - doReturn(true).when(userVmManagerImpl).expunge(vm); + List dataVolumes = new ArrayList<>(); + when(volumeDaoMock.findByInstanceAndType(vmId, Volume.Type.DATADISK)).thenReturn(dataVolumes); - try (MockedStatic mockedUsageEventUtils = Mockito.mockStatic(UsageEventUtils.class)) { + when(volumeApiService.destroyVolume(volumeId, CallContext.current().getCallingAccount(), expunge, false)).thenReturn(vol); - UserVm result = userVmManagerImpl.destroyVm(cmd); + doReturn(vm).when(userVmManagerImpl).stopVirtualMachine(anyLong(), anyBoolean()); + doReturn(vm).when(userVmManagerImpl).destroyVm(vmId, expunge); + doReturn(true).when(userVmManagerImpl).expunge(vm); - assertNotNull(result); - assertEquals(vm, result); - Mockito.verify(userVmManagerImpl).stopVirtualMachine(vmId, false); - Mockito.verify(backupManager).checkAndRemoveBackupOfferingBeforeExpunge(vm); - } + try (MockedStatic mockedUsageEventUtils = Mockito.mockStatic(UsageEventUtils.class)) { + + UserVm result = userVmManagerImpl.destroyVm(cmd); + + assertNotNull(result); + assertEquals(vm, result); + Mockito.verify(userVmManagerImpl).stopVirtualMachine(vmId, false); + Mockito.verify(backupManager).checkAndRemoveBackupOfferingBeforeExpunge(vm); } } diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerTest.java index b5d904bd2775..57aef2373576 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerTest.java @@ -85,6 +85,7 @@ import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; @@ -472,11 +473,11 @@ public void testMoveVmToUser1() throws Exception { // caller is of type 0 Account caller = new AccountVO("testaccount", 1, "networkdomain", Account.Type.NORMAL, UUID.randomUUID().toString()); - UserVO user = new UserVO(1, "testuser", "password", "firstname", "lastName", "email", "timezone", UUID.randomUUID().toString(), User.Source.UNKNOWN); - - CallContext.register(user, caller); - try { + try (MockedStatic callContextMockedStatic = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMockedStatic.when(CallContext::current).thenReturn(callContextMock); + when(callContextMock.getCallingAccount()).thenReturn(caller); _userVmMgr.moveVmToUser(cmd); } finally { CallContext.unregister(); @@ -504,7 +505,6 @@ public void testMoveVmToUser2() throws Exception { // caller is of type 0 Account caller = new AccountVO("testaccount", 1, "networkdomain", Account.Type.ADMIN, UUID.randomUUID().toString()); - UserVO user = new UserVO(1, "testuser", "password", "firstname", "lastName", "email", "timezone", UUID.randomUUID().toString(), User.Source.UNKNOWN); AccountVO oldAccount = new AccountVO("testaccount", 1, "networkdomain", Account.Type.NORMAL, UUID.randomUUID().toString()); oldAccount.setId(1L); @@ -522,14 +522,12 @@ public void testMoveVmToUser2() throws Exception { doThrow(new PermissionDeniedException("Access check failed")).when(_accountMgr).checkAccess(nullable(Account.class), nullable(AccessType.class), nullable(Boolean.class), nullable(ControlledEntity.class)); - CallContext.register(user, caller); - - when(_accountMgr.isRootAdmin(anyLong())).thenReturn(true); - - try { + try (MockedStatic callContextMockedStatic = Mockito.mockStatic(CallContext.class)) { + CallContext callContextMock = Mockito.mock(CallContext.class); + callContextMockedStatic.when(CallContext::current).thenReturn(callContextMock); + when(callContextMock.getCallingAccount()).thenReturn(caller); + when(callContextMock.isCallingAccountRootAdmin()).thenReturn(true); _userVmMgr.moveVmToUser(cmd); - } finally { - CallContext.unregister(); } } diff --git a/server/src/test/java/org/apache/cloudstack/acl/RoleManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/acl/RoleManagerImplTest.java index 5d9ee268d8b7..4298ca8d45ec 100644 --- a/server/src/test/java/org/apache/cloudstack/acl/RoleManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/acl/RoleManagerImplTest.java @@ -17,29 +17,32 @@ package org.apache.cloudstack.acl; -import com.cloud.user.Account; -import com.cloud.user.AccountManager; -import com.cloud.utils.Pair; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import org.apache.cloudstack.acl.RolePermissionEntity.Permission; import org.apache.cloudstack.acl.dao.RoleDao; import org.apache.cloudstack.acl.dao.RolePermissionsDao; +import org.apache.cloudstack.context.CallContext; import org.apache.commons.collections.CollectionUtils; +import org.junit.After; import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import com.cloud.user.Account; +import com.cloud.user.AccountManager; +import com.cloud.utils.Pair; @RunWith(MockitoJUnitRunner.class) public class RoleManagerImplTest { @@ -78,11 +81,15 @@ public class RoleManagerImplTest { private Map rolePermissions = new HashMap<>(); + private MockedStatic callContextStaticMock; + @Mock + private CallContext callContextMock; + public void setUpRoleVisibilityTests() { Mockito.doReturn(List.of("api1", "api2", "api3")).when(accountManagerMock).getApiNameList(); Mockito.doReturn(1L).when(callerAccountRoleMock).getId(); - Mockito.doReturn(callerAccountMock).when(roleManagerImpl).getCurrentAccount(); + Mockito.doReturn(callerAccountMock).when(callContextMock).getCallingAccount(); Mockito.doReturn(callerAccountRoleMock.getId()).when(callerAccountMock).getRoleId(); Mockito.when(rolePermission1Mock.getRule()).thenReturn(new Rule("api1")); @@ -105,12 +112,17 @@ public void setUpRoleVisibilityTests() { @Before public void beforeTest() { - Mockito.doReturn(accountMockId).when(accountMock).getId(); - Mockito.doReturn(accountMock).when(roleManagerImpl).getCurrentAccount(); + callContextStaticMock = Mockito.mockStatic(CallContext.class); + callContextStaticMock.when(CallContext::current).thenReturn(callContextMock); Mockito.doReturn(roleMockId).when(roleVoMock).getId(); } + @After + public void tearDown() throws Exception { + callContextStaticMock.close(); + } + @Test public void findRoleTestIdNull() { Role returnedRole = roleManagerImpl.findRole(null); @@ -140,12 +152,12 @@ public void findRoleTestRoleNotFound() { public void findRoleTestNotRootAdminAndNotRoleAdminType() { Mockito.doReturn(RoleType.DomainAdmin).when(roleVoMock).getRoleType(); Mockito.doReturn(roleVoMock).when(roleDaoMock).findById(roleMockId); - Mockito.doReturn(false).when(accountManagerMock).isRootAdmin(accountMockId); + Mockito.doReturn(false).when(callContextMock).isCallingAccountRootAdmin(); Role returnedRole = roleManagerImpl.findRole(roleMockId); Assert.assertEquals(roleMockId, returnedRole.getId()); - Mockito.verify(accountManagerMock).isRootAdmin(accountMockId); + Mockito.verify(callContextMock).isCallingAccountRootAdmin(); Mockito.verify(roleVoMock, Mockito.times(1)).getRoleType(); } @@ -153,12 +165,12 @@ public void findRoleTestNotRootAdminAndNotRoleAdminType() { public void findRoleTestRootAdminAndNotRoleAdminType() { Mockito.lenient().doReturn(RoleType.DomainAdmin).when(roleVoMock).getRoleType(); Mockito.doReturn(roleVoMock).when(roleDaoMock).findById(roleMockId); - Mockito.doReturn(true).when(accountManagerMock).isRootAdmin(accountMockId); + Mockito.doReturn(true).when(callContextMock).isCallingAccountRootAdmin(); Role returnedRole = roleManagerImpl.findRole(roleMockId); Assert.assertEquals(roleMockId, returnedRole.getId()); - Mockito.verify(accountManagerMock).isRootAdmin(accountMockId); + Mockito.verify(callContextMock).isCallingAccountRootAdmin(); Mockito.verify(roleVoMock, Mockito.times(0)).getRoleType(); } @@ -166,12 +178,12 @@ public void findRoleTestRootAdminAndNotRoleAdminType() { public void findRoleTestRootAdminAndRoleAdminType() { Mockito.lenient().doReturn(RoleType.Admin).when(roleVoMock).getRoleType(); Mockito.doReturn(roleVoMock).when(roleDaoMock).findById(roleMockId); - Mockito.doReturn(true).when(accountManagerMock).isRootAdmin(accountMockId); + Mockito.doReturn(true).when(callContextMock).isCallingAccountRootAdmin(); Role returnedRole = roleManagerImpl.findRole(roleMockId); Assert.assertEquals(roleMockId, returnedRole.getId()); - Mockito.verify(accountManagerMock).isRootAdmin(accountMockId); + Mockito.verify(callContextMock).isCallingAccountRootAdmin(); Mockito.verify(roleVoMock, Mockito.times(0)).getRoleType(); } @@ -179,12 +191,12 @@ public void findRoleTestRootAdminAndRoleAdminType() { public void findRoleTestNotRootAdminAndRoleAdminType() { Mockito.doReturn(RoleType.Admin).when(roleVoMock).getRoleType(); Mockito.doReturn(roleVoMock).when(roleDaoMock).findById(roleMockId); - Mockito.doReturn(false).when(accountManagerMock).isRootAdmin(accountMockId); + Mockito.doReturn(false).when(callContextMock).isCallingAccountRootAdmin(); Role returnedRole = roleManagerImpl.findRole(roleMockId); Assert.assertNull(returnedRole); - Mockito.verify(accountManagerMock).isRootAdmin(accountMockId); + Mockito.verify(callContextMock).isCallingAccountRootAdmin(); Mockito.verify(roleVoMock, Mockito.times(1)).getRoleType(); } @@ -322,25 +334,23 @@ public void findRolesByTypeTestNullRoleType() { List returnedRoles = roleManagerImpl.findRolesByType(null); Assert.assertEquals(0, returnedRoles.size()); - Mockito.verify(accountManagerMock, Mockito.times(0)).isRootAdmin(Mockito.anyLong()); + Mockito.verify(callContextMock, Mockito.times(0)).isCallingAccountRootAdmin(); } @Test public void findRolesByTypeTestAdminRoleNonRootAdminUser() { - Mockito.doReturn(accountMock).when(roleManagerImpl).getCurrentAccount(); - Mockito.doReturn(false).when(accountManagerMock).isRootAdmin(accountMockId); + Mockito.doReturn(false).when(callContextMock).isCallingAccountRootAdmin(); List returnedRoles = roleManagerImpl.findRolesByType(RoleType.Admin); Assert.assertEquals(0, returnedRoles.size()); - Mockito.verify(accountManagerMock, Mockito.times(1)).isRootAdmin(Mockito.anyLong()); + Mockito.verify(callContextMock, Mockito.times(1)).isCallingAccountRootAdmin(); Mockito.verify(roleDaoMock, Mockito.times(0)).findAllByRoleType(Mockito.any(RoleType.class), Mockito.anyBoolean()); } @Test public void findRolesByTypeTestAdminRoleRootAdminUser() { - Mockito.doReturn(accountMock).when(roleManagerImpl).getCurrentAccount(); - Mockito.doReturn(true).when(accountManagerMock).isRootAdmin(accountMockId); + Mockito.doReturn(true).when(callContextMock).isCallingAccountRootAdmin(); List roles = new ArrayList<>(); roles.add(Mockito.mock(Role.class)); @@ -349,13 +359,12 @@ public void findRolesByTypeTestAdminRoleRootAdminUser() { List returnedRoles = roleManagerImpl.findRolesByType(RoleType.Admin); Assert.assertEquals(1, returnedRoles.size()); - Mockito.verify(accountManagerMock, Mockito.times(2)).isRootAdmin(Mockito.anyLong()); + Mockito.verify(callContextMock, Mockito.times(2)).isCallingAccountRootAdmin(); } @Test public void findRolesByTypeTestNonAdminRoleRootAdminUser() { - Mockito.lenient().doReturn(accountMock).when(roleManagerImpl).getCurrentAccount(); - Mockito.lenient().doReturn(true).when(accountManagerMock).isRootAdmin(accountMockId); + Mockito.doReturn(true).when(callContextMock).isCallingAccountRootAdmin(); List roles = new ArrayList<>(); roles.add(Mockito.mock(Role.class)); @@ -364,7 +373,7 @@ public void findRolesByTypeTestNonAdminRoleRootAdminUser() { List returnedRoles = roleManagerImpl.findRolesByType(RoleType.User); Assert.assertEquals(1, returnedRoles.size()); - Mockito.verify(accountManagerMock, Mockito.times(1)).isRootAdmin(Mockito.anyLong()); + Mockito.verify(callContextMock, Mockito.times(1)).isCallingAccountRootAdmin(); } @Test diff --git a/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java b/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java index 61ae6cd018c7..fbdf2e4cb16d 100644 --- a/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java +++ b/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java @@ -16,9 +16,63 @@ // under the License. package org.apache.cloudstack.backup; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.TimeZone; +import java.util.UUID; + +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.command.admin.backup.ImportBackupOfferingCmd; +import org.apache.cloudstack.api.command.admin.backup.UpdateBackupOfferingCmd; +import org.apache.cloudstack.api.command.user.backup.CreateBackupCmd; +import org.apache.cloudstack.api.command.user.backup.CreateBackupScheduleCmd; +import org.apache.cloudstack.api.command.user.backup.DeleteBackupScheduleCmd; +import org.apache.cloudstack.api.response.BackupResponse; +import org.apache.cloudstack.backup.dao.BackupDao; +import org.apache.cloudstack.backup.dao.BackupDetailsDao; +import org.apache.cloudstack.backup.dao.BackupOfferingDao; +import org.apache.cloudstack.backup.dao.BackupScheduleDao; +import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.impl.ConfigDepotImpl; +import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.test.util.ReflectionTestUtils; + +import com.cloud.alert.AlertManager; import com.cloud.api.query.dao.UserVmJoinDao; import com.cloud.api.query.vo.UserVmJoinVO; -import com.cloud.alert.AlertManager; import com.cloud.capacity.CapacityVO; import com.cloud.configuration.Resource; import com.cloud.dc.DataCenter; @@ -31,6 +85,7 @@ import com.cloud.event.EventTypes; import com.cloud.event.UsageEventUtils; import com.cloud.exception.InvalidParameterValueException; +import com.cloud.exception.ResourceAllocationException; import com.cloud.exception.ResourceUnavailableException; import com.cloud.host.HostVO; import com.cloud.host.dao.HostDao; @@ -43,7 +98,6 @@ import com.cloud.service.ServiceOfferingVO; import com.cloud.service.dao.ServiceOfferingDao; import com.cloud.storage.DiskOfferingVO; -import com.cloud.exception.ResourceAllocationException; import com.cloud.storage.Storage; import com.cloud.storage.VMTemplateVO; import com.cloud.storage.Volume; @@ -66,64 +120,11 @@ import com.cloud.vm.VMInstanceDetailVO; import com.cloud.vm.VMInstanceVO; import com.cloud.vm.VirtualMachine; -import com.cloud.vm.VmDiskInfo; import com.cloud.vm.VirtualMachineManager; -import com.cloud.vm.dao.VMInstanceDetailsDao; +import com.cloud.vm.VmDiskInfo; import com.cloud.vm.dao.VMInstanceDao; +import com.cloud.vm.dao.VMInstanceDetailsDao; import com.google.gson.Gson; -import org.apache.cloudstack.api.ApiConstants; -import org.apache.cloudstack.api.ServerApiException; -import org.apache.cloudstack.api.command.admin.backup.ImportBackupOfferingCmd; -import org.apache.cloudstack.api.command.admin.backup.UpdateBackupOfferingCmd; -import org.apache.cloudstack.api.command.user.backup.CreateBackupCmd; -import org.apache.cloudstack.api.command.user.backup.CreateBackupScheduleCmd; -import org.apache.cloudstack.api.command.user.backup.DeleteBackupScheduleCmd; -import org.apache.cloudstack.api.response.BackupResponse; -import org.apache.cloudstack.backup.dao.BackupDao; -import org.apache.cloudstack.backup.dao.BackupDetailsDao; -import org.apache.cloudstack.backup.dao.BackupOfferingDao; -import org.apache.cloudstack.backup.dao.BackupScheduleDao; -import org.apache.cloudstack.context.CallContext; -import org.apache.cloudstack.framework.config.ConfigKey; -import org.apache.cloudstack.framework.config.impl.ConfigDepotImpl; -import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; -import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; -import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO; -import org.junit.After; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; -import org.mockito.InjectMocks; -import org.mockito.Mock; -import org.mockito.MockedStatic; -import org.mockito.Mockito; -import org.mockito.MockitoAnnotations; -import org.mockito.Spy; -import org.springframework.test.util.ReflectionTestUtils; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.TimeZone; -import java.util.UUID; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; -import static org.mockito.Mockito.atLeastOnce; @RunWith(MockitoJUnitRunner.class) public class BackupManagerTest { @@ -1637,8 +1638,7 @@ public void validateAndGetDefaultBackupRetentionIfRequiredTestThrowExceptionWhen when(accountVOMock.getDomainId()).thenReturn(domainId); when(domainManager.getDomain(domainId)).thenReturn(domainMock); when(resourceLimitMgr.findCorrectResourceLimitForDomain(domainMock, Resource.ResourceType.backup, null)).thenReturn(domainLimit); - when(accountVOMock.getId()).thenReturn(accountId); - when(accountManager.isRootAdmin(accountId)).thenReturn(false); + when(accountManager.isRootAdmin(accountVOMock)).thenReturn(false); backupManager.validateAndGetDefaultBackupRetentionIfRequired(maxBackups, backupOfferingVOMock, vmInstanceVOMock); } @@ -1657,8 +1657,7 @@ public void validateAndGetDefaultBackupRetentionIfRequiredTestThrowExceptionWhen when(accountVOMock.getDomainId()).thenReturn(domainId); when(domainManager.getDomain(domainId)).thenReturn(domainMock); when(resourceLimitMgr.findCorrectResourceLimitForDomain(domainMock, Resource.ResourceType.backup, null)).thenReturn(domainLimit); - when(accountVOMock.getId()).thenReturn(accountId); - when(accountManager.isRootAdmin(accountId)).thenReturn(false); + when(accountManager.isRootAdmin(accountVOMock)).thenReturn(false); backupManager.validateAndGetDefaultBackupRetentionIfRequired(maxBackups, backupOfferingVOMock, vmInstanceVOMock); } @@ -1677,8 +1676,7 @@ public void validateAndGetDefaultBackupRetentionIfRequiredTestIgnoreLimitCheckWh when(accountVOMock.getDomainId()).thenReturn(domainId); when(domainManager.getDomain(domainId)).thenReturn(domainMock); when(resourceLimitMgr.findCorrectResourceLimitForDomain(domainMock, Resource.ResourceType.backup, null)).thenReturn(domainLimit); - when(accountVOMock.getId()).thenReturn(accountId); - when(accountManager.isRootAdmin(accountId)).thenReturn(true); + when(accountManager.isRootAdmin(accountVOMock)).thenReturn(true); int retention = backupManager.validateAndGetDefaultBackupRetentionIfRequired(maxBackups, backupOfferingVOMock, vmInstanceVOMock); assertEquals(maxBackups, retention); diff --git a/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java index ec7ef20d441e..65ff3dbbe8c0 100644 --- a/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java @@ -16,20 +16,9 @@ // under the License. package org.apache.cloudstack.consoleproxy; -import com.cloud.agent.AgentManager; -import com.cloud.domain.DomainVO; -import com.cloud.domain.dao.DomainDao; -import com.cloud.exception.PermissionDeniedException; -import com.cloud.server.ManagementServer; -import com.cloud.user.Account; -import com.cloud.user.AccountManager; -import com.cloud.utils.Pair; -import com.cloud.utils.db.EntityManager; -import com.cloud.vm.ConsoleSessionVO; -import com.cloud.vm.VirtualMachine; -import com.cloud.vm.VirtualMachineManager; -import com.cloud.vm.dao.ConsoleSessionDao; -import com.cloud.vm.dao.VMInstanceDetailsDao; +import java.util.Arrays; +import java.util.List; + import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.api.ResponseGenerator; import org.apache.cloudstack.api.ResponseObject; @@ -47,8 +36,20 @@ import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; -import java.util.Arrays; -import java.util.List; +import com.cloud.agent.AgentManager; +import com.cloud.domain.DomainVO; +import com.cloud.domain.dao.DomainDao; +import com.cloud.exception.PermissionDeniedException; +import com.cloud.server.ManagementServer; +import com.cloud.user.Account; +import com.cloud.user.AccountManager; +import com.cloud.utils.Pair; +import com.cloud.utils.db.EntityManager; +import com.cloud.vm.ConsoleSessionVO; +import com.cloud.vm.VirtualMachine; +import com.cloud.vm.VirtualMachineManager; +import com.cloud.vm.dao.ConsoleSessionDao; +import com.cloud.vm.dao.VMInstanceDetailsDao; @RunWith(MockitoJUnitRunner.class) public class ConsoleAccessManagerImplTest { @@ -96,15 +97,13 @@ public class ConsoleAccessManagerImplTest { @Test public void testCheckSessionPermissionAdminAccount() { - Mockito.when(account.getId()).thenReturn(1L); - Mockito.when(accountManager.isRootAdmin(1L)).thenReturn(true); + Mockito.when(accountManager.isRootAdmin(account)).thenReturn(true); Assert.assertTrue(consoleAccessManager.checkSessionPermission(virtualMachine, account)); } @Test public void testCheckSessionPermissionUserOwnedVm() { - Mockito.when(account.getId()).thenReturn(1L); - Mockito.when(accountManager.isRootAdmin(1L)).thenReturn(false); + Mockito.when(accountManager.isRootAdmin(account)).thenReturn(false); Mockito.when(virtualMachine.getType()).thenReturn(VirtualMachine.Type.User); Mockito.doNothing().when(accountManager).checkAccess( Mockito.eq(account), Mockito.nullable(SecurityChecker.AccessType.class), @@ -115,7 +114,7 @@ public void testCheckSessionPermissionUserOwnedVm() { @Test public void testCheckSessionPermissionDifferentUserOwnedVm() { Mockito.when(account.getId()).thenReturn(1L); - Mockito.when(accountManager.isRootAdmin(1L)).thenReturn(false); + Mockito.when(accountManager.isRootAdmin(account)).thenReturn(false); Mockito.when(virtualMachine.getType()).thenReturn(VirtualMachine.Type.User); Mockito.doThrow(PermissionDeniedException.class).when(accountManager).checkAccess( Mockito.eq(account), Mockito.nullable(SecurityChecker.AccessType.class), @@ -125,8 +124,6 @@ public void testCheckSessionPermissionDifferentUserOwnedVm() { @Test public void testCheckSessionPermissionForUsersOnSystemVms() { - Mockito.when(account.getId()).thenReturn(1L); - Mockito.when(accountManager.isRootAdmin(1L)).thenReturn(false); List systemVmTypes = Arrays.asList(VirtualMachine.Type.DomainRouter, VirtualMachine.Type.ConsoleProxy, VirtualMachine.Type.SecondaryStorageVm); for (VirtualMachine.Type type : systemVmTypes) { @@ -254,8 +251,7 @@ public void listConsoleSessionsTestShouldCreateResponsesWithFullViewForRootAdmin try (MockedStatic callContextStaticMock = Mockito.mockStatic(CallContext.class)) { callContextStaticMock.when(CallContext::current).thenReturn(callContextMock); - Mockito.when(callContextMock.getCallingAccountId()).thenReturn(2L); - Mockito.when(accountManager.isRootAdmin(2L)).thenReturn(true); + Mockito.when(callContextMock.isCallingAccountRootAdmin()).thenReturn(true); Mockito.when(responseGeneratorMock.createConsoleSessionResponse(consoleSessionMock, ResponseObject.ResponseView.Full)).thenReturn(consoleSessionResponseMock); consoleAccessManager.listConsoleSessions(listConsoleSessionsCmdMock); @@ -271,8 +267,7 @@ public void listConsoleSessionsTestShouldCreateResponsesWithRestrictedViewForNon try (MockedStatic callContextStaticMock = Mockito.mockStatic(CallContext.class)) { callContextStaticMock.when(CallContext::current).thenReturn(callContextMock); - Mockito.when(callContextMock.getCallingAccountId()).thenReturn(2L); - Mockito.when(accountManager.isRootAdmin(2L)).thenReturn(false); + Mockito.when(callContextMock.isCallingAccountRootAdmin()).thenReturn(false); Mockito.when(responseGeneratorMock.createConsoleSessionResponse(consoleSessionMock, ResponseObject.ResponseView.Restricted)).thenReturn(consoleSessionResponseMock); consoleAccessManager.listConsoleSessions(listConsoleSessionsCmdMock); diff --git a/server/src/test/java/org/apache/cloudstack/storage/sharedfs/SharedFSServiceImplTest.java b/server/src/test/java/org/apache/cloudstack/storage/sharedfs/SharedFSServiceImplTest.java index 88493d10038e..626e9d1492c0 100644 --- a/server/src/test/java/org/apache/cloudstack/storage/sharedfs/SharedFSServiceImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/storage/sharedfs/SharedFSServiceImplTest.java @@ -21,8 +21,8 @@ import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; -import static org.mockito.Mockito.times; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -617,8 +617,6 @@ public void testSearchForSharedFS() { SharedFSJoinVO sharedFSJoinVO = mock(SharedFSJoinVO.class); when(sharedFSJoinDao.searchByIds(List.of(s_sharedFSId).toArray(new Long[0]))).thenReturn(List.of(sharedFSJoinVO)); - when(owner.getId()).thenReturn(s_ownerId); - when(accountMgr.isRootAdmin(any())).thenReturn(true); when(sharedFSJoinDao.createSharedFSResponses(any(), any())).thenReturn(null); ListSharedFSCmd cmd = getMockListSharedFSCmd(); From 9ec24e3710fe61605345f981d067139c473bb578 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 4 Aug 2025 23:16:31 +0530 Subject: [PATCH 2/8] fix noredist Signed-off-by: Abhishek Kumar --- .../network/contrail/management/MockAccountManager.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java index bc9dbfa7b436..85a89c8baa7e 100644 --- a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java +++ b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java @@ -230,6 +230,12 @@ public boolean isRootAdmin(Long accountId) { return false; } + @Override + public boolean isRootAdmin(Account account) { + // TODO Auto-generated method stub + return false; + } + @Override public boolean isDomainAdmin(Long accountId) { // TODO Auto-generated method stub From 3be274ce48955327565c0df76a1e36fd73e45f38 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 5 Aug 2025 10:02:39 +0530 Subject: [PATCH 3/8] fix no bean error Signed-off-by: Abhishek Kumar --- .../cloudstack/context/CallContext.java | 20 +++++++++++++------ .../context/CallContextListener.java | 6 +----- .../cloudstack/context/CallContextTest.java | 6 +----- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/context/CallContext.java b/api/src/main/java/org/apache/cloudstack/context/CallContext.java index 2b34ce934005..9e3820876c4c 100644 --- a/api/src/main/java/org/apache/cloudstack/context/CallContext.java +++ b/api/src/main/java/org/apache/cloudstack/context/CallContext.java @@ -23,8 +23,10 @@ import org.apache.cloudstack.api.ApiCommandResourceType; import org.apache.cloudstack.managed.threadlocal.ManagedThreadLocal; -import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.ThreadContext; +import org.springframework.beans.factory.NoSuchBeanDefinitionException; import com.cloud.exception.CloudAuthenticationException; import com.cloud.projects.Project; @@ -32,9 +34,9 @@ import com.cloud.user.AccountService; import com.cloud.user.User; import com.cloud.utils.UuidUtils; +import com.cloud.utils.component.ComponentContext; import com.cloud.utils.db.EntityManager; import com.cloud.utils.exception.CloudRuntimeException; -import org.apache.logging.log4j.ThreadContext; /** * CallContext records information about the environment the call is made. This @@ -69,11 +71,9 @@ protected Stack initialValue() { private String apiName; static EntityManager s_entityMgr; - static AccountService accountService; - public static void init(EntityManager entityMgr, AccountService accountService) { + public static void init(EntityManager entityMgr) { s_entityMgr = entityMgr; - CallContext.accountService = accountService; } protected CallContext() { @@ -140,7 +140,15 @@ public Account getCallingAccount() { public boolean isCallingAccountRootAdmin() { if (isAccountRootAdmin == null) { - accountService.isRootAdmin(getCallingAccount()); + AccountService accountService; + try { + accountService = ComponentContext.getDelegateComponentOfType(AccountService.class); + } catch (NoSuchBeanDefinitionException e) { + LOGGER.warn("Falling back to account type check for isRootAdmin for account ID: {} as no AccountService bean found: {}", accountId, e.getMessage()); + Account caller = getCallingAccount(); + return caller != null && caller.getType() == Account.Type.ADMIN; + } + isAccountRootAdmin = accountService.isRootAdmin(getCallingAccount()); } return Boolean.TRUE.equals(isAccountRootAdmin); } diff --git a/api/src/main/java/org/apache/cloudstack/context/CallContextListener.java b/api/src/main/java/org/apache/cloudstack/context/CallContextListener.java index c6e2ceef83b8..ab9a8c30046b 100644 --- a/api/src/main/java/org/apache/cloudstack/context/CallContextListener.java +++ b/api/src/main/java/org/apache/cloudstack/context/CallContextListener.java @@ -23,7 +23,6 @@ import org.apache.cloudstack.managed.context.ManagedContextListener; -import com.cloud.user.AccountService; import com.cloud.utils.db.EntityManager; public class CallContextListener implements ManagedContextListener { @@ -31,9 +30,6 @@ public class CallContextListener implements ManagedContextListener { @Inject EntityManager entityMgr; - @Inject - AccountService accountService; - @Override public Object onEnterContext(boolean reentry) { if (!reentry && CallContext.current() == null) { @@ -51,6 +47,6 @@ public void onLeaveContext(Object unused, boolean reentry) { @PostConstruct public void init() { - CallContext.init(entityMgr, accountService); + CallContext.init(entityMgr); } } diff --git a/api/src/test/java/org/apache/cloudstack/context/CallContextTest.java b/api/src/test/java/org/apache/cloudstack/context/CallContextTest.java index 6f76dcb54726..d3537d6f8317 100644 --- a/api/src/test/java/org/apache/cloudstack/context/CallContextTest.java +++ b/api/src/test/java/org/apache/cloudstack/context/CallContextTest.java @@ -31,7 +31,6 @@ import org.mockito.junit.MockitoJUnitRunner; import com.cloud.user.Account; -import com.cloud.user.AccountService; import com.cloud.user.User; import com.cloud.utils.db.EntityManager; @@ -41,12 +40,9 @@ public class CallContextTest { @Mock EntityManager entityMgr; - @Mock - AccountService accountService; - @Before public void setUp() { - CallContext.init(entityMgr, accountService); + CallContext.init(entityMgr); CallContext.register(Mockito.mock(User.class), Mockito.mock(Account.class)); } From e14ffd4ae067761d7e6ef19958b61ec0df5a3bef Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 5 Aug 2025 10:43:46 +0530 Subject: [PATCH 4/8] build fix Signed-off-by: Abhishek Kumar --- .../IntegrationTestConfiguration.java | 39 +++++++++---------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/IntegrationTestConfiguration.java b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/IntegrationTestConfiguration.java index 1871a90e38b4..9903f0e74901 100644 --- a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/IntegrationTestConfiguration.java +++ b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/IntegrationTestConfiguration.java @@ -21,6 +21,22 @@ import javax.inject.Inject; +import org.apache.cloudstack.framework.config.dao.ConfigurationGroupDaoImpl; +import org.apache.cloudstack.framework.config.dao.ConfigurationSubGroupDaoImpl; +import org.eclipse.jetty.security.IdentityService; +import org.mockito.ArgumentMatchers; +import org.mockito.Mockito; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.ComponentScan; +import org.springframework.context.annotation.ComponentScan.Filter; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.FilterType; +import org.springframework.core.type.classreading.MetadataReader; +import org.springframework.core.type.classreading.MetadataReaderFactory; +import org.springframework.core.type.filter.TypeFilter; + import org.apache.cloudstack.acl.APIChecker; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.affinity.AffinityGroupService; @@ -46,8 +62,6 @@ import org.apache.cloudstack.framework.config.ConfigDepot; import org.apache.cloudstack.framework.config.ConfigDepotAdmin; import org.apache.cloudstack.framework.config.dao.ConfigurationDaoImpl; -import org.apache.cloudstack.framework.config.dao.ConfigurationGroupDaoImpl; -import org.apache.cloudstack.framework.config.dao.ConfigurationSubGroupDaoImpl; import org.apache.cloudstack.framework.jobs.AsyncJobDispatcher; import org.apache.cloudstack.framework.jobs.dao.AsyncJobDaoImpl; import org.apache.cloudstack.framework.jobs.dao.AsyncJobJoinMapDaoImpl; @@ -68,24 +82,11 @@ import org.apache.cloudstack.region.dao.RegionDaoImpl; import org.apache.cloudstack.spring.lifecycle.registry.ExtensionRegistry; import org.apache.cloudstack.storage.datastore.PrimaryDataStoreProviderManager; +import org.apache.cloudstack.storage.image.datastore.ImageStoreProviderManager; import org.apache.cloudstack.storage.datastore.db.ImageStoreDaoImpl; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDaoImpl; -import org.apache.cloudstack.storage.image.datastore.ImageStoreProviderManager; import org.apache.cloudstack.storage.image.db.TemplateDataStoreDaoImpl; import org.apache.cloudstack.usage.UsageService; -import org.eclipse.jetty.security.IdentityService; -import org.mockito.ArgumentMatchers; -import org.mockito.Mockito; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.ComponentScan; -import org.springframework.context.annotation.ComponentScan.Filter; -import org.springframework.context.annotation.Configuration; -import org.springframework.context.annotation.FilterType; -import org.springframework.core.type.classreading.MetadataReader; -import org.springframework.core.type.classreading.MetadataReaderFactory; -import org.springframework.core.type.filter.TypeFilter; import com.cloud.acl.DomainChecker; import com.cloud.agent.AgentManager; @@ -267,7 +268,6 @@ import com.cloud.template.TemplateManager; import com.cloud.user.Account; import com.cloud.user.AccountDetailsDaoImpl; -import com.cloud.user.AccountService; import com.cloud.user.DomainManagerImpl; import com.cloud.user.ResourceLimitService; import com.cloud.user.User; @@ -296,8 +296,8 @@ import com.cloud.vm.dao.SecondaryStorageVmDaoImpl; import com.cloud.vm.dao.UserVmCloneSettingDaoImpl; import com.cloud.vm.dao.UserVmDaoImpl; -import com.cloud.vm.dao.VMInstanceDaoImpl; import com.cloud.vm.dao.VMInstanceDetailsDaoImpl; +import com.cloud.vm.dao.VMInstanceDaoImpl; import com.cloud.vm.snapshot.VMSnapshotManager; import com.cloud.vm.snapshot.dao.VMSnapshotDaoImpl; @@ -497,7 +497,6 @@ public EndPointSelector endPointSelector() { @Bean public EntityManager entityManager() { EntityManager mock = Mockito.mock(EntityManager.class); - AccountService accountServiceMock = Mockito.mock(AccountService.class); try { Mockito.when(mock.findById(ArgumentMatchers.same(Account.class), ArgumentMatchers.anyLong())).thenReturn(_accountDao.findById(Account.ACCOUNT_ID_SYSTEM)); Mockito.when(mock.findById(ArgumentMatchers.same(User.class), ArgumentMatchers.anyLong())).thenReturn(_userDao.findById(User.UID_SYSTEM)); @@ -525,7 +524,7 @@ public DataCenter answer(final InvocationOnMock invocation) throws Throwable { } catch (Exception e) { e.printStackTrace(); } - CallContext.init(mock, accountServiceMock); + CallContext.init(mock); return mock; } From b2d4fb646086cabbfcb278cf05fb48015e26d080 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 5 Aug 2025 14:53:26 +0530 Subject: [PATCH 5/8] fix and changes Signed-off-by: Abhishek Kumar --- .../com/cloud/api/query/QueryManagerImpl.java | 15 +++++++-------- .../ConfigurationManagerImpl.java | 5 ++--- .../com/cloud/network/vpc/VpcManagerImpl.java | 7 +++---- .../cloud/storage/VolumeApiServiceImpl.java | 8 +++----- .../storage/snapshot/SnapshotManagerImpl.java | 6 +++--- .../cloud/template/TemplateAdapterBase.java | 3 +-- .../com/cloud/usage/UsageServiceImpl.java | 2 +- .../com/cloud/user/DomainManagerImpl.java | 2 +- .../cloud/uuididentity/UUIDManagerImpl.java | 4 +--- .../java/com/cloud/vm/UserVmManagerImpl.java | 6 ++---- .../affinity/AffinityGroupServiceImpl.java | 6 +++--- .../cloud/api/query/QueryManagerImplTest.java | 1 - .../storage/snapshot/SnapshotManagerTest.java | 19 ++++++++++--------- 13 files changed, 37 insertions(+), 47 deletions(-) diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index d684cbb25586..65d507dbcfc7 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -856,7 +856,7 @@ private Pair, Integer> searchForEventsInternal(ListEventsCmd c private Pair, Integer> searchForEventIdsAndCount(ListEventsCmd cmd) { Account caller = CallContext.current().getCallingAccount(); - boolean isRootAdmin = accountMgr.isRootAdmin(caller); + boolean isRootAdmin = CallContext.current().isCallingAccountRootAdmin(); List permittedAccounts = new ArrayList<>(); Long id = cmd.getId(); @@ -950,7 +950,7 @@ private Pair, Integer> searchForEventIdsAndCount(ListEventsCmd cmd) { accountMgr.buildACLSearchCriteria(sc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); // For end users display only enabled events - if (!accountMgr.isRootAdmin(caller)) { + if (!CallContext.current().isCallingAccountRootAdmin()) { sc.setParameters("displayEvent", true); } @@ -1185,8 +1185,7 @@ public ListResponse searchForUserVMs(ListVMsCmd cmd) { } ResponseView respView = ResponseView.Restricted; - Account caller = CallContext.current().getCallingAccount(); - if (accountMgr.isRootAdmin(caller)) { + if (CallContext.current().isCallingAccountRootAdmin()) { respView = ResponseView.Full; } List vmResponses = ViewResponseHelper.createUserVmResponse(respView, "virtualmachine", cmd.getDetails(), cmd.getAccumulate(), cmd.getShowUserData(), @@ -1315,7 +1314,7 @@ private Pair, Integer> searchForUserVMIdsAndCount(ListVMsCmd cmd) { isAdmin = true; } - if (accountMgr.isRootAdmin(caller)) { + if (CallContext.current().isCallingAccountRootAdmin()) { isRootAdmin = true; podId = (Long) getObjectPossibleMethodValue(cmd, "getPodId"); clusterId = (Long) getObjectPossibleMethodValue(cmd, "getClusterId"); @@ -2630,7 +2629,7 @@ private Pair, Integer> searchForVolumeIdsAndCount(ListVolumesCmd cmd) Boolean display = cmd.getDisplay(); String state = cmd.getState(); boolean shouldListSystemVms = Boolean.TRUE.equals(cmd.getListSystemVms()) && - CallContext.registerPlaceHolderContext().isCallingAccountRootAdmin(); + CallContext.current().isCallingAccountRootAdmin(); Long zoneId = cmd.getZoneId(); Long podId = cmd.getPodId(); @@ -4009,14 +4008,14 @@ private Pair, Integer> searchForServiceOfferingIdsAndCount(ListServic final Account owner = accountMgr.finalizeOwner(caller, accountName, domainId, projectId); - if (!accountMgr.isRootAdmin(caller) && isSystem) { + if (!CallContext.current().isCallingAccountRootAdmin() && isSystem) { throw new InvalidParameterValueException("Only ROOT admins can access system offerings."); } // Keeping this logic consistent with domain specific zones // if a domainId is provided, we just return the so associated with this // domain - if (domainId != null && !accountMgr.isRootAdmin(caller)) { + if (domainId != null && !CallContext.current().isCallingAccountRootAdmin()) { // check if the user's domain == so's domain || user's domain is a // child of so's domain if (!isPermissible(owner.getDomainId(), domainId)) { diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index e9d5f92ec197..9fa6a904e69d 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -1024,7 +1024,7 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP validateIpAddressRelatedConfigValues(name, value); validateConflictingConfigValue(name, value); - if (CATEGORY_SYSTEM.equals(category) && !_accountMgr.isRootAdmin(caller)) { + if (CATEGORY_SYSTEM.equals(category) && !CallContext.current().isCallingAccountRootAdmin()) { logger.warn("Only Root Admin is allowed to edit the configuration {}", name); throw new CloudRuntimeException("Only Root Admin is allowed to edit this configuration."); } @@ -4934,9 +4934,8 @@ public Vlan createVlanAndPublicIpRange(final CreateVlanIpRangeCmd cmd) throws In } // Check if zone is enabled - final Account caller = CallContext.current().getCallingAccount(); if (Grouping.AllocationState.Disabled == zone.getAllocationState() - && !_accountMgr.isRootAdmin(caller)) { + && !CallContext.current().isCallingAccountRootAdmin()) { throw new PermissionDeniedException(String.format("Cannot perform this operation, Zone is currently disabled: %s", zone)); } diff --git a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java index ef34a6eb046f..835acfd4d2c8 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -1267,7 +1267,7 @@ public Vpc createVpc(final long zoneId, final long vpcOffId, final long vpcOwner throw new InvalidParameterValueException("Network domain must be specified for region level VPC"); } - if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller)) { + if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !CallContext.current().isCallingAccountRootAdmin()) { // See DataCenterVO.java final PermissionDeniedException ex = new PermissionDeniedException("Cannot perform this operation since specified Zone is currently disabled"); ex.addProxyObject(zone.getUuid(), "zoneId"); @@ -2643,8 +2643,7 @@ private void validateVpcPrivateGatewayAssociateNetworkId(NetworkOfferingVO ntwkO if (broadcastUri != null && associatedNetworkId != null) { throw new InvalidParameterValueException("vlanId and associatedNetworkId are mutually exclusive"); } - Account caller = CallContext.current().getCallingAccount(); - if (!_accountMgr.isRootAdmin(caller) && (ntwkOff.isSpecifyVlan() || broadcastUri != null || bypassVlanOverlapCheck)) { + if (!CallContext.current().isCallingAccountRootAdmin() && (ntwkOff.isSpecifyVlan() || broadcastUri != null || bypassVlanOverlapCheck)) { throw new InvalidParameterValueException("Only ROOT admin is allowed to specify vlanId or bypass vlan overlap check"); } if (ntwkOff.isSpecifyVlan() && broadcastUri == null) { @@ -2767,7 +2766,7 @@ public boolean deleteVpcPrivateGateway(final long gatewayId) throws ConcurrentOp } final Account caller = CallContext.current().getCallingAccount(); - if (!_accountMgr.isRootAdmin(caller)) { + if (!CallContext.current().isCallingAccountRootAdmin()) { _accountMgr.checkAccess(caller, null, false, gatewayVO); final NetworkVO networkVO = _ntwkDao.findById(gatewayVO.getNetworkId()); if (networkVO != null) { diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 1783d901f0ed..f6274c347331 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -946,7 +946,7 @@ public VolumeVO allocVolume(CreateVolumeCmd cmd) throws ResourceAllocationExcept } // Check if zone is disabled - if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller)) { + if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !CallContext.current().isCallingAccountRootAdmin()) { throw new PermissionDeniedException(String.format("Cannot perform this operation, Zone: %s is currently disabled", zone)); } @@ -3989,8 +3989,6 @@ private boolean isOperationSupported(VMTemplateVO template, UserVmVO userVm) { @Override @ActionEvent(eventType = EventTypes.EVENT_SNAPSHOT_CREATE, eventDescription = "allocating snapshot", create = true) public Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName, Snapshot.LocationType locationType, List zoneIds, List poolIds, Boolean useStorageReplication) throws ResourceAllocationException { - Account caller = CallContext.current().getCallingAccount(); - VolumeInfo volume = volFactory.getVolume(volumeId); if (volume == null) { throw new InvalidParameterValueException("Creating snapshot failed due to volume:" + volumeId + " doesn't exist"); @@ -4059,7 +4057,7 @@ public Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName, if (dataCenter == null) { throw new InvalidParameterValueException("Unable to find the specified zone"); } - if (Grouping.AllocationState.Disabled.equals(dataCenter.getAllocationState()) && !_accountMgr.isRootAdmin(caller)) { + if (Grouping.AllocationState.Disabled.equals(dataCenter.getAllocationState()) && !CallContext.current().isCallingAccountRootAdmin()) { throw new PermissionDeniedException("Cannot perform this operation, Zone is currently disabled: " + dataCenter.getName()); } if (DataCenter.Type.Edge.equals(dataCenter.getType())) { @@ -4115,7 +4113,7 @@ public Snapshot allocSnapshotForVm(Long vmId, Long volumeId, String snapshotName throw new InvalidParameterValueException("Can't find zone by id " + volume.getDataCenterId()); } - if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller)) { + if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !CallContext.current().isCallingAccountRootAdmin()) { throw new PermissionDeniedException("Cannot perform this operation, Zone is currently disabled: " + zone.getName()); } diff --git a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java index ba8b902e9e0c..24b2634a32fc 100755 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -533,7 +533,7 @@ public String extractSnapshot(ExtractSnapshotCmd cmd) { Long snapshotId = cmd.getId(); Long zoneId = cmd.getZoneId(); - if (!_accountMgr.isRootAdmin(caller) && ApiDBUtils.isExtractionDisabled()) { + if (!CallContext.current().isCallingAccountRootAdmin() && ApiDBUtils.isExtractionDisabled()) { logger.error("Extraction is disabled through [{}].", Config.DisableExtraction); throw new PermissionDeniedException("Extraction could not be completed."); } @@ -2264,9 +2264,9 @@ public Snapshot copySnapshot(CopySnapshotCmd cmd) throws StorageUnavailableExcep storagePoolIds = snapshotHelper.addStoragePoolsForCopyToPrimary(volume, destZoneIds, storagePoolIds, useStorageReplication); boolean canCopyBetweenStoragePools = CollectionUtils.isNotEmpty(storagePoolIds) && canCopyOnPrimary(storagePoolIds, snapshotVO); Map dataCenterVOs = new HashMap<>(); - boolean isRootAdminCaller = _accountMgr.isRootAdmin(caller); for (Long destZoneId: destZoneIds) { - DataCenterVO dstZone = getCheckedDestinationZoneForSnapshotCopy(destZoneId, isRootAdminCaller); + DataCenterVO dstZone = getCheckedDestinationZoneForSnapshotCopy(destZoneId, + CallContext.current().isCallingAccountRootAdmin()); dataCenterVOs.put(destZoneId, dstZone); } _accountMgr.checkAccess(caller, SecurityChecker.AccessType.OperateEntry, true, snapshot); diff --git a/server/src/main/java/com/cloud/template/TemplateAdapterBase.java b/server/src/main/java/com/cloud/template/TemplateAdapterBase.java index 1c510baa8fac..91e31434922f 100644 --- a/server/src/main/java/com/cloud/template/TemplateAdapterBase.java +++ b/server/src/main/java/com/cloud/template/TemplateAdapterBase.java @@ -371,8 +371,7 @@ public TemplateProfile prepare(boolean isIso, long userId, String name, String d if (zone == null) { throw new IllegalArgumentException("Please specify a valid zone."); } - Account caller = CallContext.current().getCallingAccount(); - if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller)) { + if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !CallContext.current().isCallingAccountRootAdmin()) { throw new PermissionDeniedException(String.format("Cannot perform this operation, Zone %s is currently disabled", zone)); } } diff --git a/server/src/main/java/com/cloud/usage/UsageServiceImpl.java b/server/src/main/java/com/cloud/usage/UsageServiceImpl.java index 4d16a1e15da8..84d2b036db1f 100644 --- a/server/src/main/java/com/cloud/usage/UsageServiceImpl.java +++ b/server/src/main/java/com/cloud/usage/UsageServiceImpl.java @@ -193,7 +193,7 @@ public Pair, Integer> getUsageRecords(ListUsageRecordsCmd accountId = caller.getId(); //List records for all the accounts if the caller account is of type admin. //If account_id or account_name is explicitly mentioned, list records for the specified account only even if the caller is of type admin - ignoreAccountId = _accountService.isRootAdmin(caller); + ignoreAccountId = CallContext.current().isCallingAccountRootAdmin(); logger.debug("Account details not available. Using userContext account: {}", caller); } diff --git a/server/src/main/java/com/cloud/user/DomainManagerImpl.java b/server/src/main/java/com/cloud/user/DomainManagerImpl.java index 7134790f76ed..f0279ac27780 100644 --- a/server/src/main/java/com/cloud/user/DomainManagerImpl.java +++ b/server/src/main/java/com/cloud/user/DomainManagerImpl.java @@ -721,7 +721,7 @@ public Pair, Integer> searchForDomains(ListDomainsCmd cmd } _accountMgr.checkAccess(caller, domain); } else { - if (!_accountMgr.isRootAdmin(caller)) { + if (!CallContext.current().isCallingAccountRootAdmin()) { domainId = caller.getDomainId(); } if (listAll) { diff --git a/server/src/main/java/com/cloud/uuididentity/UUIDManagerImpl.java b/server/src/main/java/com/cloud/uuididentity/UUIDManagerImpl.java index a5afeeeb01f4..1ccd00ddeee6 100644 --- a/server/src/main/java/com/cloud/uuididentity/UUIDManagerImpl.java +++ b/server/src/main/java/com/cloud/uuididentity/UUIDManagerImpl.java @@ -47,10 +47,8 @@ public void checkUuid(String uuid, Class entityType) { return; } - Account caller = CallContext.current().getCallingAccount(); - // Only admin and system allowed to do this - if (!(caller.getId() == Account.ACCOUNT_ID_SYSTEM || _accountMgr.isRootAdmin(caller))) { + if (!(CallContext.current().getCallingAccountId() == Account.ACCOUNT_ID_SYSTEM || CallContext.current().isCallingAccountRootAdmin())) { throw new PermissionDeniedException("Please check your permissions, you are not allowed to create/update custom id"); } diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index c3e03312b664..802490ed24fe 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -6930,8 +6930,7 @@ public VirtualMachine getVm(long vmId) { private VMInstanceVO preVmStorageMigrationCheck(Long vmId) { // access check - only root admin can migrate VM - Account caller = CallContext.current().getCallingAccount(); - if (!_accountMgr.isRootAdmin(caller)) { + if (!CallContext.current().isCallingAccountRootAdmin()) { if (logger.isDebugEnabled()) { logger.debug("Caller is not a root admin, permission denied to migrate the VM"); } @@ -7072,8 +7071,7 @@ public boolean isVMUsingLocalStorage(VMInstanceVO vm) { public VirtualMachine migrateVirtualMachine(Long vmId, Host destinationHost) throws ResourceUnavailableException, ConcurrentOperationException, ManagementServerException, VirtualMachineMigrationException { // access check - only root admin can migrate VM - Account caller = CallContext.current().getCallingAccount(); - if (!_accountMgr.isRootAdmin(caller)) { + if (!CallContext.current().isCallingAccountRootAdmin()) { if (logger.isDebugEnabled()) { logger.debug("Caller is not a root admin, permission denied to migrate the VM"); } diff --git a/server/src/main/java/org/apache/cloudstack/affinity/AffinityGroupServiceImpl.java b/server/src/main/java/org/apache/cloudstack/affinity/AffinityGroupServiceImpl.java index ffdf95e77198..2185b95cb99f 100644 --- a/server/src/main/java/org/apache/cloudstack/affinity/AffinityGroupServiceImpl.java +++ b/server/src/main/java/org/apache/cloudstack/affinity/AffinityGroupServiceImpl.java @@ -133,7 +133,7 @@ public AffinityGroup createAffinityGroup(final String accountName, final Long pr } Account caller = CallContext.current().getCallingAccount(); - if (processor.isAdminControlledGroup() && !_accountMgr.isRootAdmin(caller)) { + if (processor.isAdminControlledGroup() && !CallContext.current().isCallingAccountRootAdmin()) { throw new PermissionDeniedException("Cannot create the affinity group"); } @@ -464,7 +464,7 @@ public UserVm updateVMAffinityGroups(Long vmId, List affinityGroupIds) { if (ag.getAclType() == ACLType.Domain) { _accountMgr.checkAccess(caller, null, false, owner, ag); // make sure the affinity group is available in that domain - if (caller.getId() == Account.ACCOUNT_ID_SYSTEM || _accountMgr.isRootAdmin(caller)) { + if (caller.getId() == Account.ACCOUNT_ID_SYSTEM || CallContext.current().isCallingAccountRootAdmin()) { if (!isAffinityGroupAvailableInDomain(ag.getId(), owner.getDomainId())) { throw new PermissionDeniedException("Affinity Group " + ag + " does not belong to the VM's domain"); } @@ -474,7 +474,7 @@ public UserVm updateVMAffinityGroups(Long vmId, List affinityGroupIds) { // Root admin has access to both VM and AG by default, // but // make sure the owner of these entities is same - if (caller.getId() == Account.ACCOUNT_ID_SYSTEM || _accountMgr.isRootAdmin(caller)) { + if (caller.getId() == Account.ACCOUNT_ID_SYSTEM || CallContext.current().isCallingAccountRootAdmin()) { if (ag.getAccountId() != owner.getAccountId()) { throw new PermissionDeniedException("Affinity Group " + ag + " does not belong to the VM's account"); } diff --git a/server/src/test/java/com/cloud/api/query/QueryManagerImplTest.java b/server/src/test/java/com/cloud/api/query/QueryManagerImplTest.java index 7da491265ef9..496cbc791bf0 100644 --- a/server/src/test/java/com/cloud/api/query/QueryManagerImplTest.java +++ b/server/src/test/java/com/cloud/api/query/QueryManagerImplTest.java @@ -196,7 +196,6 @@ private void setupCommonMocks() { user = new UserVO(1, "testuser", "password", "firstname", "lastName", "email", "timezone", UUID.randomUUID().toString(), User.Source.UNKNOWN); CallContext.register(user, account); - Mockito.when(accountManager.isRootAdmin(account)).thenReturn(false); final SearchBuilder eventSearchBuilder = mock(SearchBuilder.class); final SearchCriteria eventSearchCriteria = mock(SearchCriteria.class); final EventVO eventVO = mock(EventVO.class); diff --git a/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java b/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java index 4bfcffea71e9..deaed01b727f 100755 --- a/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java +++ b/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java @@ -32,7 +32,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.UUID; import org.apache.cloudstack.api.command.user.snapshot.ExtractSnapshotCmd; import org.apache.cloudstack.context.CallContext; @@ -95,8 +94,6 @@ import com.cloud.user.AccountManager; import com.cloud.user.AccountVO; import com.cloud.user.ResourceLimitService; -import com.cloud.user.User; -import com.cloud.user.UserVO; import com.cloud.utils.DateUtil; import com.cloud.utils.DateUtil.IntervalType; import com.cloud.utils.db.GlobalLock; @@ -193,6 +190,8 @@ public class SnapshotManagerTest { ImageStoreEntity imageStoreEntityMock; @Mock DataStoreManager dataStoreManagerMock; + @Mock + private CallContext callContextMock; SnapshotPolicyVO snapshotPolicyVoInstance; @@ -215,9 +214,12 @@ public class SnapshotManagerTest { private static final String TEST_SNAPSHOT_PATH = "path"; private static final Storage.ImageFormat TEST_VOLUME_FORMAT = Storage.ImageFormat.RAW; + MockedStatic callContextMockedStatic; + @Before public void setup() throws ResourceAllocationException { - + callContextMockedStatic = Mockito.mockStatic(CallContext.class); + callContextMockedStatic.when(CallContext::current).thenReturn(callContextMock); when(_snapshotDao.findById(anyLong())).thenReturn(snapshotMock); when(snapshotMock.getVolumeId()).thenReturn(TEST_VOLUME_ID); @@ -239,8 +241,7 @@ public void setup() throws ResourceAllocationException { doNothing().when(_resourceLimitMgr).incrementResourceCount(anyLong(), any(ResourceType.class), anyLong()); Account account = new AccountVO("testaccount", 1L, "networkdomain", Account.Type.NORMAL, "uuid"); - UserVO user = new UserVO(1, "testuser", "password", "firstname", "lastName", "email", "timezone", UUID.randomUUID().toString(), User.Source.UNKNOWN); - CallContext.register(user, account); + when(callContextMock.getCallingAccount()).thenReturn(account); when(_accountMgr.getAccount(anyLong())).thenReturn(account); when(_storagePoolDao.findById(anyLong())).thenReturn(poolMock); @@ -257,7 +258,7 @@ public void setup() throws ResourceAllocationException { @After public void tearDown() throws Exception { apiDBUtilsMock.close(); - CallContext.unregister(); + callContextMockedStatic.close(); } // vm is destroyed @@ -561,7 +562,7 @@ public void testIsBackupSnapshotToSecondaryForEdgeZone() { private void mockForExtractSnapshotTests() { Mockito.doReturn(TEST_SNAPSHOT_ID).when(extractSnapshotCmdMock).getId(); Mockito.doReturn(TEST_ZONE_ID).when(extractSnapshotCmdMock).getZoneId(); - Mockito.doReturn(false).when(_accountMgr).isRootAdmin(any(Account.class)); + Mockito.doReturn(false).when(callContextMock).isCallingAccountRootAdmin(); Mockito.when(ApiDBUtils.isExtractionDisabled()).thenReturn(false); Mockito.doReturn(dataCenterVOMock).when(dataCenterDao).findById(TEST_ZONE_ID); @@ -654,7 +655,7 @@ public void extractSnapshotTestCreateExtractUrlReturnUrl() { @Test() public void extractSnapshotTestRootAdminDisabledExtractionCreateExtractUrlReturnUrl() { mockForExtractSnapshotTests(); - Mockito.doReturn(true).when(_accountMgr).isRootAdmin(any(Account.class)); + Mockito.doReturn(true).when(callContextMock).isCallingAccountRootAdmin(); Mockito.when(ApiDBUtils.isExtractionDisabled()).thenReturn(true); Assert.assertEquals(TEST_EXTRACT_URL, _snapshotMgr.extractSnapshot(extractSnapshotCmdMock)); From ef5cf762debf2014c4c126548a635a697a7dba51 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 5 Aug 2025 17:49:59 +0530 Subject: [PATCH 6/8] add tests Signed-off-by: Abhishek Kumar --- .../cloudstack/context/CallContextTest.java | 55 ++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/api/src/test/java/org/apache/cloudstack/context/CallContextTest.java b/api/src/test/java/org/apache/cloudstack/context/CallContextTest.java index d3537d6f8317..179212c4e581 100644 --- a/api/src/test/java/org/apache/cloudstack/context/CallContextTest.java +++ b/api/src/test/java/org/apache/cloudstack/context/CallContextTest.java @@ -27,11 +27,14 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; +import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; import com.cloud.user.Account; +import com.cloud.user.AccountService; import com.cloud.user.User; +import com.cloud.utils.component.ComponentContext; import com.cloud.utils.db.EntityManager; @RunWith(MockitoJUnitRunner.class) @@ -39,11 +42,15 @@ public class CallContextTest { @Mock EntityManager entityMgr; + @Mock + User user; + @Mock + Account account; @Before public void setUp() { CallContext.init(entityMgr); - CallContext.register(Mockito.mock(User.class), Mockito.mock(Account.class)); + CallContext.register(user, account); } @After @@ -80,4 +87,50 @@ public void testGetContextParameter() { Assert.assertEquals("current context map should have exactly three entries", 3, currentContext.getContextParameters().size()); } + + @Test + public void isCallingAccountRootAdminReturnsTrueWhenAccountIsRootAdminAccountServiceNotAvailable() { + Mockito.when(account.getType()).thenReturn(Account.Type.ADMIN); + + CallContext context = CallContext.current(); + Assert.assertTrue(context.isCallingAccountRootAdmin()); + } + + @Test + public void isCallingAccountRootAdminReturnsFalseWhenAccountIsNotRootAdminAccountServiceNotAvailable() { + Mockito.when(account.getType()).thenReturn(Account.Type.NORMAL); + + CallContext context = CallContext.current(); + Assert.assertFalse(context.isCallingAccountRootAdmin()); + Assert.assertFalse(context.isCallingAccountRootAdmin()); + } + + @Test + public void isCallingAccountRootAdminTrueWhenAccountServiceAvailable() { + try (MockedStatic componentContextMockedStatic = Mockito.mockStatic(ComponentContext.class)) { + AccountService accountService = Mockito.mock(AccountService.class); + Mockito.when(accountService.isRootAdmin(account)).thenReturn(true); + componentContextMockedStatic.when(() -> ComponentContext.getDelegateComponentOfType(AccountService.class)).thenReturn(accountService); + CallContext context = CallContext.current(); + Assert.assertTrue(context.isCallingAccountRootAdmin()); + // Verify isRootAdmin was called only once + Assert.assertTrue(context.isCallingAccountRootAdmin()); + componentContextMockedStatic.verify(() -> ComponentContext.getDelegateComponentOfType(AccountService.class)); + } + } + + @Test + public void isCallingAccountRootAdminFalseWhenAccountServiceAvailable() { + try (MockedStatic componentContextMockedStatic = Mockito.mockStatic(ComponentContext.class)) { + AccountService accountService = Mockito.mock(AccountService.class); + Mockito.when(accountService.isRootAdmin(account)).thenReturn(false); + componentContextMockedStatic.when(() -> ComponentContext.getDelegateComponentOfType(AccountService.class)).thenReturn(accountService); + CallContext context = CallContext.current(); + Assert.assertFalse(context.isCallingAccountRootAdmin()); + // Verify isRootAdmin was called only once + Assert.assertFalse(context.isCallingAccountRootAdmin()); + componentContextMockedStatic.verify(() -> ComponentContext.getDelegateComponentOfType(AccountService.class)); + } + } + } From ffddb7f33e1cee038089c05e6f3872899a9ad37e Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 5 Aug 2025 18:02:38 +0530 Subject: [PATCH 7/8] more test Signed-off-by: Abhishek Kumar --- .../cloud/user/AccountManagerImplTest.java | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java index 49243ce9f6d0..32e9ca7c4369 100644 --- a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java +++ b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java @@ -23,6 +23,7 @@ import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -31,6 +32,7 @@ import org.apache.cloudstack.acl.Role; import org.apache.cloudstack.acl.RoleService; import org.apache.cloudstack.acl.RoleType; +import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.api.command.admin.account.UpdateAccountCmd; import org.apache.cloudstack.api.command.admin.user.DeleteUserCmd; @@ -1591,4 +1593,71 @@ public void testcheckCallerApiPermissionsForUserOperationsNotAllowedApis() { accountManagerImpl.checkCallerApiPermissionsForUserOrAccountOperations(accountMock); } } + + @Test + public void isRootAdminReturnsTrueWhenCheckerGrantsAccess() { + Account account = Mockito.mock(Account.class); + SecurityChecker checker = Mockito.mock(SecurityChecker.class); + Mockito.when(checker.checkAccess(account, null, null, "SystemCapability")).thenReturn(true); + List securityCheckers = List.of(checker); + accountManagerImpl.setSecurityCheckers(securityCheckers); + boolean result = accountManagerImpl.isRootAdmin(account); + + Assert.assertTrue(result); + } + + @Test + public void isRootAdminReturnsFalseWhenCheckerDeniesAccess() { + Account account = Mockito.mock(Account.class); + SecurityChecker checker = Mockito.mock(SecurityChecker.class); + Mockito.when(checker.checkAccess(account, null, null, "SystemCapability")).thenThrow(PermissionDeniedException.class); + List securityCheckers = List.of(checker); + accountManagerImpl.setSecurityCheckers(securityCheckers); + boolean result = accountManagerImpl.isRootAdmin(account); + + Assert.assertFalse(result); + } + + @Test + public void isRootAdminReturnsFalseWhenAccountIsNull() { + Account account = null; + boolean result = accountManagerImpl.isRootAdmin(account); + Assert.assertFalse(result); + } + + @Test + public void isRootAdminReturnsFalseWhenNoCheckersExist() { + Account account = Mockito.mock(Account.class); + accountManagerImpl.setSecurityCheckers(Collections.emptyList()); + boolean result = accountManagerImpl.isRootAdmin(account); + Assert.assertFalse(result); + } + + @Test + public void isRootAdminReturnsTrueWhenMultipleCheckersGrantAccess() { + Account account = Mockito.mock(Account.class); + SecurityChecker checker1 = Mockito.mock(SecurityChecker.class); + SecurityChecker checker2 = Mockito.mock(SecurityChecker.class); + Mockito.when(checker1.checkAccess(account, null, null, "SystemCapability")).thenReturn(false); + Mockito.when(checker2.checkAccess(account, null, null, "SystemCapability")).thenReturn(true); + List securityCheckers = List.of(checker1, checker2); + accountManagerImpl.setSecurityCheckers(securityCheckers); + boolean result = accountManagerImpl.isRootAdmin(account); + + Assert.assertTrue(result); + } + + @Test + public void isRootAdminReturnsFalseWhenSecondCheckerDenyAccess() { + Account account = Mockito.mock(Account.class); + SecurityChecker checker1 = Mockito.mock(SecurityChecker.class); + SecurityChecker checker2 = Mockito.mock(SecurityChecker.class); + Mockito.when(checker1.checkAccess(account, null, null, "SystemCapability")).thenReturn(false); + Mockito.when(checker2.checkAccess(account, null, null, "SystemCapability")).thenThrow(PermissionDeniedException.class); + List securityCheckers = List.of(checker1, checker2); + accountManagerImpl.setSecurityCheckers(securityCheckers); + boolean result = accountManagerImpl.isRootAdmin(account); + + Assert.assertFalse(result); + } } From c82622194aadb7d5fbf13725a96ff8e6c3e4b019 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 7 Aug 2025 16:48:55 +0530 Subject: [PATCH 8/8] add javadoc Signed-off-by: Abhishek Kumar --- api/src/main/java/com/cloud/user/AccountService.java | 6 ++++++ server/src/main/java/com/cloud/user/AccountManagerImpl.java | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/api/src/main/java/com/cloud/user/AccountService.java b/api/src/main/java/com/cloud/user/AccountService.java index 4a2b5108962a..caec789dd66e 100644 --- a/api/src/main/java/com/cloud/user/AccountService.java +++ b/api/src/main/java/com/cloud/user/AccountService.java @@ -85,6 +85,12 @@ User createUser(String userName, String password, String firstName, String lastN boolean isRootAdmin(Long accountId); + /** + * Checks if the given account has ROOT admin privileges. + * + * @param account the account to check + * @return true if the account is a ROOT admin, false otherwise + */ boolean isRootAdmin(Account account); boolean isDomainAdmin(Long accountId); diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index b9f41d84a224..5c34bdacd7c8 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -606,7 +606,7 @@ public boolean isRootAdmin(Long accountId) { @Override public boolean isRootAdmin(Account account) { if (account == null) { - return false; //account is deleted or does not exist + return false; } for (SecurityChecker checker : _securityCheckers) { try {