server: fix potential NPE while ldap authentication (#3418)

This fixes a potential NPE when a mapped account is not found and
moving of user to the mapped account is performed. This will now
throw a more information exception than NPE.

Fixes #2853

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

Author: yadvr

plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java

@@ -316,7 +316,7 @@ public boolean moveUser(MoveUserCmd moveUserCmd) {
     }
 
     @Override
-    public boolean moveUser(long id, Long domainId, long accountId) {
+    public boolean moveUser(long id, Long domainId, Account account) {
         return false;
     }
 

plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapAuthenticator.java

@@ -35,6 +35,7 @@
 import com.cloud.user.dao.UserAccountDao;
 import com.cloud.utils.Pair;
 import com.cloud.utils.component.AdapterBase;
+import com.cloud.utils.exception.CloudRuntimeException;
 
 public class LdapAuthenticator extends AdapterBase implements UserAuthenticator {
     private static final Logger s_logger = Logger.getLogger(LdapAuthenticator.class.getName());
@@ -135,7 +136,11 @@ private Pair<Boolean, ActionOnFailedAuthentication> authenticate(String username
                 } else {
                     // not a new user, check if mapped group has changed
                     if(userAccount.getAccountId() != mapping.getAccountId()) {
-                        _accountManager.moveUser(userAccount.getId(),userAccount.getDomainId(),mapping.getAccountId());
+                        final Account mappedAccount = _accountManager.getAccount(mapping.getAccountId());
+                        if (mappedAccount == null || mappedAccount.getRemoved() != null) {
+                            throw new CloudRuntimeException("Mapped account for users does not exist. Please contact your administrator.");
+                        }
+                        _accountManager.moveUser(userAccount.getId(), userAccount.getDomainId(), mappedAccount);
                     }
                     // else { the user hasn't changed in ldap, the ldap group stayed the same, hurray, pass, fun thou self a lot of fun }
                 }

server/src/main/java/com/cloud/user/AccountManager.java

@@ -180,11 +180,12 @@ void buildACLViewSearchCriteria(SearchCriteria<? extends ControlledViewEntity> s
 
     List<String> listAclGroupsByAccount(Long accountId);
 
-    public static final String MESSAGE_ADD_ACCOUNT_EVENT = "Message.AddAccount.Event";
+    String MESSAGE_ADD_ACCOUNT_EVENT = "Message.AddAccount.Event";
 
-    public static final String MESSAGE_REMOVE_ACCOUNT_EVENT = "Message.RemoveAccount.Event";
-    public static final ConfigKey<Boolean> UseSecretKeyInResponse = new ConfigKey<Boolean>("Advanced", Boolean.class, "use.secret.key.in.response", "false",
+    String MESSAGE_REMOVE_ACCOUNT_EVENT = "Message.RemoveAccount.Event";
+
+    ConfigKey<Boolean> UseSecretKeyInResponse = new ConfigKey<Boolean>("Advanced", Boolean.class, "use.secret.key.in.response", "false",
             "This parameter allows the users to enable or disable of showing secret key as a part of response for various APIs. By default it is set to false.", true);
 
-    boolean moveUser(long id, Long domainId, long accountId);
+    boolean moveUser(long id, Long domainId, Account newAccount);
 }

server/src/main/java/com/cloud/user/AccountManagerImpl.java

@@ -1817,13 +1817,12 @@ public boolean moveUser(MoveUserCmd cmd) {
     }
 
     @Override
-    public boolean moveUser(long id, Long domainId, long accountId) {
+    public boolean moveUser(long id, Long domainId, Account newAccount) {
         UserVO user = getValidUserVO(id);
         Account oldAccount = _accountDao.findById(user.getAccountId());
         checkAccountAndAccess(user, oldAccount);
-        Account newAccount = _accountDao.findById(accountId);
         checkIfNotMovingAcrossDomains(domainId, newAccount);
-        return moveUser(user, accountId);
+        return moveUser(user, newAccount.getId());
     }
 
     private boolean moveUser(UserVO user, long newAccountId) {

server/src/test/java/com/cloud/user/MockAccountManagerImpl.java

@@ -129,7 +129,7 @@ public boolean moveUser(MoveUserCmd moveUserCmd) {
     }
 
     @Override
-    public boolean moveUser(long id, Long domainId, long accountId) {
+    public boolean moveUser(long id, Long domainId, Account account) {
         return false;
     }