Skip to content

Handle listProjects API to list projects with user as members when listAll=true #4316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yadvr merged 3 commits into from
Sep 17, 2020
Merged

Handle listProjects API to list projects with user as members when listAll=true #4316

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
61f2dc2
added defensive checks for avoiding NPE and list projects API fix
Sep 10, 2020
b9443fd
Merge branch 'master' of https://github.com/apache/cloudstack into li…
Sep 10, 2020
63c516a
list projects with account name provided to not include users in the …
Sep 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion server/src/main/java/com/cloud/acl/AffinityGroupAccessChecker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions server/src/main/java/com/cloud/acl/DomainChecker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Copy link
Member

@yadvr yadvr Sep 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there usage of this method where throwing an exception would break flow? (or, are we expecting all usages of this method to be project related?)

Copy link
Contributor Author

@Pearl1594 Pearl1594 Sep 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this method is called only if the entity is owned by a project - which would mean that ideally project should be present if the entity's account is of Project type

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();

Expand Down
10 changes: 7 additions & 3 deletions server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1479,15 +1479,19 @@ private Pair<List<ProjectJoinVO>, 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) {
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<ProjectJoinVO> sc = sb.create();
Expand Down
3 changes: 3 additions & 0 deletions server/src/main/java/com/cloud/network/NetworkModelImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Member

@yadvr yadvr Sep 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as previous comment

Copy link
Contributor Author

@Pearl1594 Pearl1594 Sep 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here too, the check is explicitly for networks owned by a project

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())) {
Expand Down
11 changes: 8 additions & 3 deletions server/src/main/java/com/cloud/projects/ProjectManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down