This is an automated email from the ASF dual-hosted git repository. rohit pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/master by this push: new 0833cf1 server: fix potential NPE while ldap authentication (#3418) 0833cf1 is described below commit 0833cf1dd74b03fd2aa805588f05bf555e1ceaf0 Author: Rohit Yadav <rohit.ya...@shapeblue.com> AuthorDate: Wed Jun 26 10:27:21 2019 +0530 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.ya...@shapeblue.com> --- .../network/contrail/management/MockAccountManager.java | 2 +- .../main/java/org/apache/cloudstack/ldap/LdapAuthenticator.java | 7 ++++++- server/src/main/java/com/cloud/user/AccountManager.java | 9 +++++---- server/src/main/java/com/cloud/user/AccountManagerImpl.java | 5 ++--- server/src/test/java/com/cloud/user/MockAccountManagerImpl.java | 2 +- 5 files changed, 15 insertions(+), 10 deletions(-) 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 100f380..f07a743 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 @@ -316,7 +316,7 @@ public class MockAccountManager extends ManagerBase implements AccountManager { } @Override - public boolean moveUser(long id, Long domainId, long accountId) { + public boolean moveUser(long id, Long domainId, Account account) { return false; } diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapAuthenticator.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapAuthenticator.java index 517c718..2d8fe53 100644 --- a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapAuthenticator.java +++ b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapAuthenticator.java @@ -35,6 +35,7 @@ import com.cloud.user.UserAccount; 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 @@ public class LdapAuthenticator extends AdapterBase implements UserAuthenticator } 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 } } diff --git a/server/src/main/java/com/cloud/user/AccountManager.java b/server/src/main/java/com/cloud/user/AccountManager.java index de6dcca..57012e1 100644 --- a/server/src/main/java/com/cloud/user/AccountManager.java +++ b/server/src/main/java/com/cloud/user/AccountManager.java @@ -180,11 +180,12 @@ public interface AccountManager extends AccountService, Configurable { 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); } diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index bda4cca..b71d548 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -1817,13 +1817,12 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M } @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) { diff --git a/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java b/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java index 4fbf752..9fece09 100644 --- a/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java +++ b/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java @@ -129,7 +129,7 @@ public class MockAccountManagerImpl extends ManagerBase implements Manager, Acco } @Override - public boolean moveUser(long id, Long domainId, long accountId) { + public boolean moveUser(long id, Long domainId, Account account) { return false; }