Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,13 @@ public interface NetworkPermissionDao extends GenericDao<NetworkPermissionVO, Lo
*/
void removeAllPermissions(long networkId);

/**
* Removes all network permissions associated with a given account.
*
* @param accountId The ID of the account from which all network permissions will be removed.
*/
void removeAccountPermissions(long accountId);

/**
* Find a Network permission by networkId, accountName, and domainId
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@

private SearchBuilder<NetworkPermissionVO> NetworkAndAccountSearch;
private SearchBuilder<NetworkPermissionVO> NetworkIdSearch;
private SearchBuilder<NetworkPermissionVO> accountSearch;
private GenericSearchBuilder<NetworkPermissionVO, Long> FindNetworkIdsByAccount;

protected NetworkPermissionDaoImpl() {
Expand All @@ -47,6 +48,10 @@
NetworkIdSearch.and("networkId", NetworkIdSearch.entity().getNetworkId(), SearchCriteria.Op.EQ);
NetworkIdSearch.done();

accountSearch = createSearchBuilder();
accountSearch.and("accountId", accountSearch.entity().getAccountId(), SearchCriteria.Op.EQ);
accountSearch.done();

Check warning on line 53 in engine/schema/src/main/java/org/apache/cloudstack/network/dao/NetworkPermissionDaoImpl.java

View check run for this annotation

Codecov / codecov/patch

engine/schema/src/main/java/org/apache/cloudstack/network/dao/NetworkPermissionDaoImpl.java#L51-L53

Added lines #L51 - L53 were not covered by tests

FindNetworkIdsByAccount = createSearchBuilder(Long.class);
FindNetworkIdsByAccount.select(null, SearchCriteria.Func.DISTINCT, FindNetworkIdsByAccount.entity().getNetworkId());
FindNetworkIdsByAccount.and("account", FindNetworkIdsByAccount.entity().getAccountId(), SearchCriteria.Op.IN);
Expand All @@ -71,6 +76,16 @@
expunge(sc);
}

@Override
public void removeAccountPermissions(long accountId) {
SearchCriteria<NetworkPermissionVO> sc = accountSearch.create();
sc.setParameters("accountId", accountId);
int networkPermissionRemoved = expunge(sc);

Check warning on line 83 in engine/schema/src/main/java/org/apache/cloudstack/network/dao/NetworkPermissionDaoImpl.java

View check run for this annotation

Codecov / codecov/patch

engine/schema/src/main/java/org/apache/cloudstack/network/dao/NetworkPermissionDaoImpl.java#L80-L83

Added lines #L80 - L83 were not covered by tests
if (networkPermissionRemoved > 0) {
s_logger.debug(String.format("Removed [%s] network permission(s) for the account with Id [%s]", networkPermissionRemoved, accountId));

Check warning on line 85 in engine/schema/src/main/java/org/apache/cloudstack/network/dao/NetworkPermissionDaoImpl.java

View check run for this annotation

Codecov / codecov/patch

engine/schema/src/main/java/org/apache/cloudstack/network/dao/NetworkPermissionDaoImpl.java#L85

Added line #L85 was not covered by tests
}
}

Check warning on line 87 in engine/schema/src/main/java/org/apache/cloudstack/network/dao/NetworkPermissionDaoImpl.java

View check run for this annotation

Codecov / codecov/patch

engine/schema/src/main/java/org/apache/cloudstack/network/dao/NetworkPermissionDaoImpl.java#L87

Added line #L87 was not covered by tests

@Override
public NetworkPermissionVO findByNetworkAndAccount(long networkId, long accountId) {
SearchCriteria<NetworkPermissionVO> sc = NetworkAndAccountSearch.create();
Expand Down
29 changes: 18 additions & 11 deletions server/src/main/java/com/cloud/user/AccountManagerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
import org.apache.cloudstack.framework.messagebus.MessageBus;
import org.apache.cloudstack.framework.messagebus.PublishScope;
import org.apache.cloudstack.managed.context.ManagedContextRunnable;
import org.apache.cloudstack.network.dao.NetworkPermissionDao;
import org.apache.cloudstack.region.gslb.GlobalLoadBalancerRuleDao;
import org.apache.cloudstack.resourcedetail.UserDetailVO;
import org.apache.cloudstack.resourcedetail.dao.UserDetailsDao;
Expand Down Expand Up @@ -298,6 +299,8 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
private SSHKeyPairDao _sshKeyPairDao;
@Inject
private UserDataDao userDataDao;
@Inject
private NetworkPermissionDao networkPermissionDao;

private List<QuerySelector> _querySelectors;

Expand Down Expand Up @@ -870,6 +873,9 @@ protected boolean cleanupAccount(AccountVO account, long callerUserId, Account c
// delete the account from project accounts
_projectAccountDao.removeAccountFromProjects(accountId);

// Delete account's network permissions
networkPermissionDao.removeAccountPermissions(accountId);

if (account.getType() != Account.Type.PROJECT) {
// delete the account from group
_messageBus.publish(_name, MESSAGE_REMOVE_ACCOUNT_EVENT, PublishScope.LOCAL, accountId);
Expand Down Expand Up @@ -1857,26 +1863,27 @@ public boolean deleteUserAccount(long accountId) {
// If the user is a System user, return an error. We do not allow this
AccountVO account = _accountDao.findById(accountId);

if (! isDeleteNeeded(account, accountId, caller)) {
if (!isDeleteNeeded(account, accountId, caller)) {
return true;
}

// Account that manages project(s) can't be removed
List<Long> managedProjectIds = _projectAccountDao.listAdministratedProjectIds(accountId);
if (!managedProjectIds.isEmpty()) {
StringBuilder projectIds = new StringBuilder();
for (Long projectId : managedProjectIds) {
projectIds.append(projectId).append(", ");
}

throw new InvalidParameterValueException("The account id=" + accountId + " manages project(s) with ids " + projectIds + "and can't be removed");
}
checkIfAccountManagesProjects(accountId);

CallContext.current().putContextParameter(Account.class, account.getUuid());

return deleteAccount(account, callerUserId, caller);
}

protected void checkIfAccountManagesProjects(long accountId) {
List<Long> managedProjectIds = _projectAccountDao.listAdministratedProjectIds(accountId);
if (!CollectionUtils.isEmpty(managedProjectIds)) {
throw new InvalidParameterValueException(String.format(
"Unable to delete account [%s], because it manages the following project(s): %s. Please, remove the account from these projects or demote it to a regular project role first.",
accountId, managedProjectIds
));
}
}

private boolean isDeleteNeeded(AccountVO account, long accountId, Account caller) {
if (account == null) {
s_logger.info(String.format("The account, identified by id %d, doesn't exist", accountId ));
Expand Down
18 changes: 18 additions & 0 deletions server/src/test/java/com/cloud/user/AccountManagerImplTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1200,4 +1200,22 @@ public void testValidateRoleAdminCannotEscalateAdminFromNonRootDomain() {
Mockito.when(roleService.findRole(2L)).thenReturn(callerRole);
accountManagerImpl.validateRoleChange(account, newRole, caller);
}

@Test
public void checkIfAccountManagesProjectsTestNotThrowExceptionWhenTheAccountIsNotAProjectAdministrator() {
long accountId = 1L;
List<Long> managedProjectIds = new ArrayList<>();

Mockito.when(_projectAccountDao.listAdministratedProjectIds(accountId)).thenReturn(managedProjectIds);
accountManagerImpl.checkIfAccountManagesProjects(accountId);
}

@Test(expected = InvalidParameterValueException.class)
public void checkIfAccountManagesProjectsTestThrowExceptionWhenTheAccountIsAProjectAdministrator() {
long accountId = 1L;
List<Long> managedProjectIds = List.of(1L);

Mockito.when(_projectAccountDao.listAdministratedProjectIds(accountId)).thenReturn(managedProjectIds);
accountManagerImpl.checkIfAccountManagesProjects(accountId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
import org.apache.cloudstack.engine.service.api.OrchestrationService;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.cloudstack.framework.messagebus.MessageBus;
import org.apache.cloudstack.network.dao.NetworkPermissionDao;
import org.apache.cloudstack.region.gslb.GlobalLoadBalancerRuleDao;
import org.apache.cloudstack.resourcedetail.dao.UserDetailsDao;
import org.junit.After;
Expand Down Expand Up @@ -195,6 +196,8 @@ public class AccountManagetImplTestBase {
SSHKeyPairDao _sshKeyPairDao;
@Mock
UserDataDao userDataDao;
@Mock
NetworkPermissionDao networkPermissionDaoMock;

@Spy
@InjectMocks
Expand Down