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;
}
