From d5acabdbf7c2b49db3bc35f8434724b9a979843b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gabriel=20Beims=20Br=C3=A4scher?= <gabriel@apache.org>
Date: Tue, 1 Sep 2020 05:28:42 -0300
Subject: [PATCH] server: Avoid Null pointer at DomainChecker and enhance
 AssignVMCmd (#4279)

When executing request assignVirtualMachine with null domainID and a valid projectID then a NullPointerException happens at DomainChecker.java.

Command example:

assign virtualmachine virtualmachineid=vmID projectid=projectID account=admin
The NullPointerException that is thrown at DomainChecker is handled at AssignVMCmd.java#L142, resulting in the following log message: Failed to move vm null.
---
 .../cloudstack/api/command/admin/vm/AssignVMCmd.java   | 10 ++++++++--
 server/src/main/java/com/cloud/acl/DomainChecker.java  |  5 +++++
 .../src/main/java/com/cloud/vm/UserVmManagerImpl.java  |  4 ++++
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java
index da5f68860bc..7b577963f1b 100644
--- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java
+++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java
@@ -138,8 +138,14 @@ public class AssignVMCmd extends BaseCmd  {
             e.printStackTrace();
             throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, e.getMessage());
         } catch (Exception e) {
-            e.printStackTrace();
-            throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to move vm " + e.getMessage());
+            s_logger.error("Failed to move vm due to: " + e.getStackTrace());
+            if (e.getMessage() != null) {
+                throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to move vm due to " + e.getMessage());
+            } else if (e.getCause() != null) {
+                throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to move vm due to " + e.getCause());
+            } else {
+                throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to move vm");
+            }
         }
 
     }
diff --git a/server/src/main/java/com/cloud/acl/DomainChecker.java b/server/src/main/java/com/cloud/acl/DomainChecker.java
index 1077d2bc480..5fc2b343be9 100644
--- a/server/src/main/java/com/cloud/acl/DomainChecker.java
+++ b/server/src/main/java/com/cloud/acl/DomainChecker.java
@@ -106,6 +106,11 @@ public class DomainChecker extends AdapterBase implements SecurityChecker {
         if (caller.getState() != Account.State.enabled) {
             throw new PermissionDeniedException(caller + " is disabled.");
         }
+
+        if (domain == null) {
+            throw new PermissionDeniedException(String.format("Provided domain is NULL, cannot check access for account [uuid=%s, name=%s]", caller.getUuid(), caller.getAccountName()));
+        }
+
         long domainId = domain.getId();
 
         if (_accountService.isNormalUser(caller.getId())) {
diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
index 802ac1e0dd7..f99a0663e29 100644
--- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
@@ -6133,6 +6133,10 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
             throw new InvalidParameterValueException("The new account owner " + cmd.getAccountName() + " is disabled.");
         }
 
+        if (cmd.getProjectId() != null && cmd.getDomainId() == null) {
+            throw new InvalidParameterValueException("Please provide a valid domain ID; cannot assign VM to a project if domain ID is NULL.");
+        }
+
         //check caller has access to both the old and new account
         _accountMgr.checkAccess(caller, null, true, oldAccount);
         _accountMgr.checkAccess(caller, null, true, newAccount);