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 000ee36 CLOUDSTACK-9971: Bugfix/listaccounts parameter consistency
(#2156)
000ee36 is described below
commit 000ee36224feb81fb86da2ffebb812f4d3d1e35a
Author: Daniel Carbone <[email protected]>
AuthorDate: Wed Jan 3 05:29:54 2018 -0600
CLOUDSTACK-9971: Bugfix/listaccounts parameter consistency (#2156)
Ran into an issue today where we passed both the "id" and "domainid"
parameters into "listAccounts" and received a response despite the account id
passed not belonging to the domainid passed.
Allow usage of "domainid" AND "id" in "listAccounts"
- Adding "AccountDoa::findActiveAccountById"
- Adding "AccountDaoImpl::findActiveAccountById"
- Removing seemingly pointless "listForDomain" parameter
- Updating "typeNEQ" value from "5" to "Account.ACCOUNT_TYPE_PROJECT"
(which is "5")
- Only attempt to load domain for "path" query parameter once
"searchForAccountsInternal" input validation logic pseudo-code:
- If "domainid" set, check immediately
- If "id" not set:
- and user is admin and "listall" is true
- if "domainid" not set, use caller domain id
- force "isrecursive" true
- else use caller account id
- Else if "domainid" and "name" set
- verify existence of account and that user has access
- Else:
- if "domainid" not set, locate account by "id"
- else, locate account by "id" and "domainid"
- verify account found and caller has access rights
---
.../schema/src/com/cloud/user/dao/AccountDao.java | 8 ++-
.../src/com/cloud/user/dao/AccountDaoImpl.java | 23 ++++---
.../src/com/cloud/api/query/QueryManagerImpl.java | 80 +++++++++++++---------
3 files changed, 66 insertions(+), 45 deletions(-)
diff --git a/engine/schema/src/com/cloud/user/dao/AccountDao.java
b/engine/schema/src/com/cloud/user/dao/AccountDao.java
index 374c9cc..b97f318 100644
--- a/engine/schema/src/com/cloud/user/dao/AccountDao.java
+++ b/engine/schema/src/com/cloud/user/dao/AccountDao.java
@@ -16,9 +16,6 @@
// under the License.
package com.cloud.user.dao;
-import java.util.Date;
-import java.util.List;
-
import com.cloud.user.Account;
import com.cloud.user.AccountVO;
import com.cloud.user.User;
@@ -26,6 +23,9 @@ import com.cloud.utils.Pair;
import com.cloud.utils.db.Filter;
import com.cloud.utils.db.GenericDao;
+import java.util.Date;
+import java.util.List;
+
public interface AccountDao extends GenericDao<AccountVO, Long> {
Pair<User, Account> findUserAccountByApiKey(String apiKey);
@@ -62,6 +62,8 @@ public interface AccountDao extends GenericDao<AccountVO,
Long> {
//returns only non-removed account
Account findActiveAccount(String accountName, Long domainId);
+ Account findActiveAccountById(Long accountId, Long domainId);
+
Account findActiveNonProjectAccount(String accountName, Long domainId);
List<Long> getAccountIdsForDomains(List<Long> ids);
diff --git a/engine/schema/src/com/cloud/user/dao/AccountDaoImpl.java
b/engine/schema/src/com/cloud/user/dao/AccountDaoImpl.java
index 2789150..956f8a8 100644
--- a/engine/schema/src/com/cloud/user/dao/AccountDaoImpl.java
+++ b/engine/schema/src/com/cloud/user/dao/AccountDaoImpl.java
@@ -16,15 +16,6 @@
// under the License.
package com.cloud.user.dao;
-import java.sql.PreparedStatement;
-import java.sql.ResultSet;
-import java.util.Date;
-import java.util.List;
-
-
-import org.apache.log4j.Logger;
-import org.springframework.stereotype.Component;
-
import com.cloud.user.Account;
import com.cloud.user.Account.State;
import com.cloud.user.AccountVO;
@@ -39,6 +30,13 @@ import com.cloud.utils.db.SearchBuilder;
import com.cloud.utils.db.SearchCriteria;
import com.cloud.utils.db.SearchCriteria.Op;
import com.cloud.utils.db.TransactionLegacy;
+import org.apache.log4j.Logger;
+import org.springframework.stereotype.Component;
+
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.util.Date;
+import java.util.List;
@Component
public class AccountDaoImpl extends GenericDaoBase<AccountVO, Long> implements
AccountDao {
@@ -189,6 +187,13 @@ public class AccountDaoImpl extends
GenericDaoBase<AccountVO, Long> implements A
}
@Override
+ public Account findActiveAccountById(Long accountId, Long domainId) {
+ SearchCriteria<AccountVO> sc = AllFieldsSearch.create("id", accountId);
+ sc.setParameters("domainId", domainId);
+ return findOneBy(sc);
+ }
+
+ @Override
public Account findActiveNonProjectAccount(String accountName, Long
domainId) {
SearchCriteria<AccountVO> sc =
NonProjectAccountSearch.create("accountName", accountName);
sc.setParameters("domainId", domainId);
diff --git a/server/src/com/cloud/api/query/QueryManagerImpl.java
b/server/src/com/cloud/api/query/QueryManagerImpl.java
index 2a6919b..2d6fabc 100644
--- a/server/src/com/cloud/api/query/QueryManagerImpl.java
+++ b/server/src/com/cloud/api/query/QueryManagerImpl.java
@@ -1982,47 +1982,58 @@ public class QueryManagerImpl extends
MutualExclusiveIdsManagerBase implements Q
String accountName = cmd.getSearchName();
boolean isRecursive = cmd.isRecursive();
boolean listAll = cmd.listAll();
- Boolean listForDomain = false;
-
- if (accountId != null) {
- Account account = _accountDao.findById(accountId);
- if (account == null || account.getId() ==
Account.ACCOUNT_ID_SYSTEM) {
- throw new InvalidParameterValueException("Unable to find
account by id " + accountId);
- }
-
- _accountMgr.checkAccess(caller, null, true, account);
- }
+ boolean callerIsAdmin = _accountMgr.isAdmin(caller.getId());
+ Account account;
+ Domain domain = null;
+ // if "domainid" specified, perform validation
if (domainId != null) {
- Domain domain = _domainDao.findById(domainId);
+ // ensure existence...
+ domain = _domainDao.findById(domainId);
if (domain == null) {
throw new InvalidParameterValueException("Domain id=" +
domainId + " doesn't exist");
}
-
+ // ... and check access rights.
_accountMgr.checkAccess(caller, domain);
-
- if (accountName != null) {
- Account account = _accountDao.findActiveAccount(accountName,
domainId);
- if (account == null || account.getId() ==
Account.ACCOUNT_ID_SYSTEM) {
- throw new InvalidParameterValueException("Unable to find
account by name " + accountName
- + " in domain " + domainId);
- }
- _accountMgr.checkAccess(caller, null, true, account);
- }
}
+ // if no "id" specified...
if (accountId == null) {
- if (_accountMgr.isAdmin(caller.getId()) && listAll && domainId ==
null) {
- listForDomain = true;
- isRecursive = true;
+ // listall only has significance if they are an admin
+ if (listAll && callerIsAdmin) {
+ // if no domain id specified, use caller's domain
if (domainId == null) {
domainId = caller.getDomainId();
}
- } else if (_accountMgr.isAdmin(caller.getId()) && domainId !=
null) {
- listForDomain = true;
- } else {
+ // mark recursive
+ isRecursive = true;
+ } else if (!callerIsAdmin || domainId == null) {
accountId = caller.getAccountId();
}
+ } else if (domainId != null && accountName != null) {
+ // if they're looking for an account by name
+ account = _accountDao.findActiveAccount(accountName, domainId);
+ if (account == null || account.getId() ==
Account.ACCOUNT_ID_SYSTEM) {
+ throw new InvalidParameterValueException(
+ "Unable to find account by name " + accountName + " in
domain " + domainId
+ );
+ }
+ _accountMgr.checkAccess(caller, null, true, account);
+ } else {
+ // if they specified an "id"...
+ if (domainId == null) {
+ account = _accountDao.findById(accountId);
+ } else {
+ account = _accountDao.findActiveAccountById(accountId,
domainId);
+ }
+ if (account == null || account.getId() ==
Account.ACCOUNT_ID_SYSTEM) {
+ throw new InvalidParameterValueException(
+ "Unable to find account by id "
+ + accountId
+ + (domainId == null ? "" : " in domain " +
domainId)
+ );
+ }
+ _accountMgr.checkAccess(caller, null, true, account);
}
Filter searchFilter = new Filter(AccountJoinVO.class, "id", true,
cmd.getStartIndex(), cmd.getPageSizeVal());
@@ -2042,12 +2053,15 @@ public class QueryManagerImpl extends
MutualExclusiveIdsManagerBase implements Q
sb.and("typeNEQ", sb.entity().getType(), SearchCriteria.Op.NEQ);
sb.and("idNEQ", sb.entity().getId(), SearchCriteria.Op.NEQ);
- if (listForDomain && isRecursive) {
+ if (domainId != null && isRecursive) {
sb.and("path", sb.entity().getDomainPath(),
SearchCriteria.Op.LIKE);
}
SearchCriteria<AccountJoinVO> sc = sb.create();
+ // don't return account of type project to the end user
+ sc.setParameters("typeNEQ", Account.ACCOUNT_TYPE_PROJECT);
+ // don't return system account...
sc.setParameters("idNEQ", Account.ACCOUNT_ID_SYSTEM);
if (keyword != null) {
@@ -2073,16 +2087,16 @@ public class QueryManagerImpl extends
MutualExclusiveIdsManagerBase implements Q
sc.setParameters("accountName", accountName);
}
- // don't return account of type project to the end user
- sc.setParameters("typeNEQ", 5);
-
if (accountId != null) {
sc.setParameters("id", accountId);
}
- if (listForDomain) {
+ if (domainId != null) {
if (isRecursive) {
- Domain domain = _domainDao.findById(domainId);
+ // will happen if no "domainid" was specified in the request...
+ if (domain == null) {
+ domain = _domainDao.findById(domainId);
+ }
sc.setParameters("path", domain.getPath() + "%");
} else {
sc.setParameters("domainId", domainId);
--
To stop receiving notification emails like this one, please contact
['"[email protected]" <[email protected]>'].
