From 61f2dc29f77e4f9120666ba032b20d07eb6a60e3 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 10 Sep 2020 11:56:58 +0530 Subject: [PATCH 1/2] added defensive checks for avoiding NPE and list projects API fix --- .../com/cloud/acl/AffinityGroupAccessChecker.java | 2 +- server/src/main/java/com/cloud/acl/DomainChecker.java | 4 ++++ .../java/com/cloud/api/query/QueryManagerImpl.java | 2 -- .../main/java/com/cloud/network/NetworkModelImpl.java | 3 +++ .../java/com/cloud/projects/ProjectManagerImpl.java | 11 ++++++++--- 5 files changed, 16 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/com/cloud/acl/AffinityGroupAccessChecker.java b/server/src/main/java/com/cloud/acl/AffinityGroupAccessChecker.java index 6106c7268e13..3a648cdcbf0a 100644 --- a/server/src/main/java/com/cloud/acl/AffinityGroupAccessChecker.java +++ b/server/src/main/java/com/cloud/acl/AffinityGroupAccessChecker.java @@ -80,8 +80,8 @@ public boolean checkAccess(Account caller, ControlledEntity entity, AccessType a //check if the group belongs to a project User user = CallContext.current().getCallingUser(); ProjectVO project = _projectDao.findByProjectAccountId(group.getAccountId()); - ProjectAccount userProjectAccount = _projectAccountDao.findByProjectIdUserId(project.getId(), user.getAccountId(), user.getId()); if (project != null) { + ProjectAccount userProjectAccount = _projectAccountDao.findByProjectIdUserId(project.getId(), user.getAccountId(), user.getId()); if (userProjectAccount != null) { if (AccessType.ModifyProject.equals(accessType) && _projectAccountDao.canUserModifyProject(project.getId(), user.getAccountId(), user.getId())) { return true; diff --git a/server/src/main/java/com/cloud/acl/DomainChecker.java b/server/src/main/java/com/cloud/acl/DomainChecker.java index 5fc2b343be9e..24b6b2a42b42 100644 --- a/server/src/main/java/com/cloud/acl/DomainChecker.java +++ b/server/src/main/java/com/cloud/acl/DomainChecker.java @@ -61,6 +61,7 @@ import com.cloud.user.User; import com.cloud.user.dao.AccountDao; import com.cloud.utils.component.AdapterBase; +import com.cloud.utils.exception.CloudRuntimeException; @Component public class DomainChecker extends AdapterBase implements SecurityChecker { @@ -199,6 +200,9 @@ public boolean checkAccess(Account caller, ControlledEntity entity, AccessType a private boolean checkOperationPermitted(Account caller, ControlledEntity entity) { User user = CallContext.current().getCallingUser(); Project project = projectDao.findByProjectAccountId(entity.getAccountId()); + if (project == null) { + throw new CloudRuntimeException("Unable to find project to which the entity belongs to"); + } ProjectAccount projectUser = _projectAccountDao.findByProjectIdUserId(project.getId(), user.getAccountId(), user.getId()); String apiCommandName = CallContext.current().getApiName(); diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index f13b7953e336..83ad0cd08453 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -1486,8 +1486,6 @@ private Pair, Integer> listProjectsInternal(ListProjectsCmd sb.and().op("userId", sb.entity().getUserId(), Op.EQ); sb.or("userIdNull", sb.entity().getUserId(), Op.NULL); sb.cp(); - } else { - sb.and("userIdNull", sb.entity().getUserId(), Op.NULL); } SearchCriteria sc = sb.create(); diff --git a/server/src/main/java/com/cloud/network/NetworkModelImpl.java b/server/src/main/java/com/cloud/network/NetworkModelImpl.java index aabcf2b10bfc..b6eab90a98cf 100644 --- a/server/src/main/java/com/cloud/network/NetworkModelImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkModelImpl.java @@ -1658,6 +1658,9 @@ public void checkNetworkPermissions(Account owner, Network network) { if (owner.getType() != Account.ACCOUNT_TYPE_PROJECT && networkOwner.getType() == Account.ACCOUNT_TYPE_PROJECT) { User user = CallContext.current().getCallingUser(); Project project = projectDao.findByProjectAccountId(network.getAccountId()); + if (project == null) { + throw new CloudRuntimeException("Unable to find project to which the network belongs to"); + } ProjectAccount projectAccountUser = _projectAccountDao.findByProjectIdUserId(project.getId(), user.getAccountId(), user.getId()); if (projectAccountUser != null) { if (!_projectAccountDao.canUserAccessProjectAccount(user.getAccountId(), user.getId(), network.getAccountId())) { diff --git a/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java b/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java index 90a27fcafd01..88ad0c2ffc9d 100644 --- a/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java +++ b/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java @@ -239,6 +239,9 @@ public Project createProject(final String name, final String displayText, String } User user = validateUser(userId, accountId, domainId); + if (user != null) { + owner = _accountDao.findById(user.getAccountId()); + } //do resource limit check _resourceLimitMgr.checkResourceLimit(owner, ResourceType.project); @@ -559,9 +562,11 @@ public boolean canAccessProjectAccount(Account caller, long accountId) { } User user = CallContext.current().getCallingUser(); ProjectVO project = _projectDao.findByProjectAccountId(accountId); - ProjectAccount userProjectAccount = _projectAccountDao.findByProjectIdUserId(project.getId(), user.getAccountId(), user.getId()); - if (userProjectAccount != null) { - return _projectAccountDao.canUserAccessProjectAccount(user.getAccountId(), user.getId(), accountId); + if (project != null) { + ProjectAccount userProjectAccount = _projectAccountDao.findByProjectIdUserId(project.getId(), user.getAccountId(), user.getId()); + if (userProjectAccount != null) { + return _projectAccountDao.canUserAccessProjectAccount(user.getAccountId(), user.getId(), accountId); + } } return _projectAccountDao.canAccessProjectAccount(caller.getId(), accountId); } From 63c516afda6ef602ea7a754f92750a39beaf1086 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Fri, 11 Sep 2020 09:19:34 +0530 Subject: [PATCH 2/2] list projects with account name provided to not include users in the account in response --- .../main/java/com/cloud/api/query/QueryManagerImpl.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index 83ad0cd08453..871029fba8d1 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -1479,7 +1479,13 @@ private Pair, Integer> listProjectsInternal(ListProjectsCmd } if (accountId != null) { - sb.and("accountId", sb.entity().getAccountId(), SearchCriteria.Op.EQ); + if (userId == null) { + sb.and().op("accountId", sb.entity().getAccountId(), SearchCriteria.Op.EQ); + sb.and("userIdNull", sb.entity().getUserId(), Op.NULL); + sb.cp(); + } else { + sb.and("accountId", sb.entity().getAccountId(), SearchCriteria.Op.EQ); + } } if (userId != null) {