[
https://issues.apache.org/jira/browse/CLOUDSTACK-9971?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16309513#comment-16309513
]
ASF GitHub Bot commented on CLOUDSTACK-9971:
--------------------------------------------
rhtyd closed pull request #2156: CLOUDSTACK-9971: Bugfix/listaccounts parameter
consistency
URL: https://github.com/apache/cloudstack/pull/2156
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/engine/schema/src/com/cloud/user/dao/AccountDao.java
b/engine/schema/src/com/cloud/user/dao/AccountDao.java
index 4c7ce8e31b7..72b31eb4cad 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.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);
@@ -60,6 +60,8 @@
//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 bff6213c10b..4eae2871617 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.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 {
@@ -182,6 +180,13 @@ public Account findActiveAccount(String accountName, Long
domainId) {
return findOneBy(sc);
}
+ @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);
diff --git a/server/src/com/cloud/api/query/QueryManagerImpl.java
b/server/src/com/cloud/api/query/QueryManagerImpl.java
index 17c5855694e..4e0dade63ea 100644
--- a/server/src/com/cloud/api/query/QueryManagerImpl.java
+++ b/server/src/com/cloud/api/query/QueryManagerImpl.java
@@ -1953,47 +1953,58 @@
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());
@@ -2013,12 +2024,15 @@
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) {
@@ -2044,16 +2058,16 @@
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);
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Bugfix/listaccounts parameter consistency
> -----------------------------------------
>
> Key: CLOUDSTACK-9971
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9971
> Project: CloudStack
> Issue Type: Bug
> Security Level: Public(Anyone can view this level - this is the
> default.)
> Reporter: Daniel Carbone
> Priority: Minor
>
> Ran into an issue 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.
> This PR aims to correct this issue, along with a few other small updates.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
