From f88f934274793174ed72164ffed8562003d1a28b Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 10 Feb 2022 10:07:23 +0530 Subject: [PATCH] api, server: fix add-remove vpn user without vpn owner (#5850) * api, server: fix add-remove vpn user without vpn owner Fixes #5711 ACS should not add a new user in Add state when the owner account does not have VPN access. While removing VPN user ACS should not fail completely when owner account ahs no VPN. * change , fixes * remove unused method Signed-off-by: Abhishek Kumar --- .../network/vpn/RemoteAccessVpnService.java | 2 + .../command/user/vpn/RemoveVpnUserCmd.java | 3 +- .../vpn/RemoteAccessVpnManagerImpl.java | 58 ++++++++++++++++--- 3 files changed, 52 insertions(+), 11 deletions(-) diff --git a/api/src/main/java/com/cloud/network/vpn/RemoteAccessVpnService.java b/api/src/main/java/com/cloud/network/vpn/RemoteAccessVpnService.java index 5426d181e70..bbb9771d27a 100644 --- a/api/src/main/java/com/cloud/network/vpn/RemoteAccessVpnService.java +++ b/api/src/main/java/com/cloud/network/vpn/RemoteAccessVpnService.java @@ -43,6 +43,8 @@ public interface RemoteAccessVpnService { List listVpnUsers(long vpnOwnerId, String userName); + boolean applyVpnUsers(long vpnOwnerId, String userName, boolean forRemove) throws ResourceUnavailableException; + boolean applyVpnUsers(long vpnOwnerId, String userName) throws ResourceUnavailableException; Pair, Integer> searchForRemoteAccessVpns(ListRemoteAccessVpnsCmd cmd); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vpn/RemoveVpnUserCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vpn/RemoveVpnUserCmd.java index 33cbb46485c..caeb0608b6a 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vpn/RemoveVpnUserCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vpn/RemoveVpnUserCmd.java @@ -120,9 +120,8 @@ public class RemoveVpnUserCmd extends BaseAsyncCmd { } boolean appliedVpnUsers = false; - try { - appliedVpnUsers = _ravService.applyVpnUsers(ownerId, userName); + appliedVpnUsers = _ravService.applyVpnUsers(ownerId, userName, true); } catch (ResourceUnavailableException ex) { String errorMessage = String.format("Failed to refresh VPN user=[%s] due to resource unavailable. VPN owner id=[%s].", userName, ownerId); s_logger.error(errorMessage, ex); diff --git a/server/src/main/java/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java b/server/src/main/java/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java index 74b53f14f07..61d247d7b8a 100644 --- a/server/src/main/java/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java @@ -16,16 +16,16 @@ // under the License. package com.cloud.network.vpn; +import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import javax.inject.Inject; import javax.naming.ConfigurationException; -import org.apache.log4j.Logger; - import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.api.command.user.vpn.ListRemoteAccessVpnsCmd; import org.apache.cloudstack.api.command.user.vpn.ListVpnUsersCmd; @@ -33,6 +33,8 @@ import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.Configurable; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.apache.commons.collections.CollectionUtils; +import org.apache.log4j.Logger; import com.cloud.configuration.Config; import com.cloud.domain.DomainVO; @@ -91,9 +93,6 @@ import com.cloud.utils.db.TransactionCallbackWithException; import com.cloud.utils.db.TransactionStatus; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.net.NetUtils; -import java.lang.reflect.InvocationTargetException; -import java.util.stream.Collectors; -import org.apache.commons.collections.CollectionUtils; public class RemoteAccessVpnManagerImpl extends ManagerBase implements RemoteAccessVpnService, Configurable { private final static Logger s_logger = Logger.getLogger(RemoteAccessVpnManagerImpl.class); @@ -138,6 +137,24 @@ public class RemoteAccessVpnManagerImpl extends ManagerBase implements RemoteAcc int _pskLength; SearchBuilder VpnSearch; + private List getValidRemoteAccessVpnForAccount(long accountId) { + List vpns = _remoteAccessVpnDao.findByAccount(accountId); + if (CollectionUtils.isNotEmpty(vpns)) { + List validVpns = new ArrayList<>(); + for (RemoteAccessVpnVO vpn : vpns) { + if (vpn.getNetworkId() != null) { + Network network = _networkMgr.getNetwork(vpn.getNetworkId()); + if (!Network.State.Implemented.equals(network.getState())) { + continue; + } + } + validVpns.add(vpn); + } + vpns = validVpns; + } + return vpns; + } + @Override @DB public RemoteAccessVpn createRemoteAccessVpn(final long publicIpId, String ipRange, boolean openFirewall, final Boolean forDisplay) throws NetworkRuleConflictException { @@ -499,19 +516,36 @@ public class RemoteAccessVpnManagerImpl extends ManagerBase implements RemoteAcc } } + @DB + private boolean removeVpnUserWithoutRemoteAccessVpn(long vpnOwnerId, String userName) { + VpnUserVO vpnUser = _vpnUsersDao.findByAccountAndUsername(vpnOwnerId, userName); + if (vpnUser == null) { + s_logger.error(String.format("VPN user not found with ownerId: %d and username: %s", vpnOwnerId, userName)); + return false; + } + if (!State.Revoke.equals(vpnUser.getState())) { + s_logger.error(String.format("VPN user with ownerId: %d and username: %s is not in revoked state, current state: %s", vpnOwnerId, userName, vpnUser.getState())); + return false; + } + return _vpnUsersDao.remove(vpnUser.getId()); + } + @DB @Override - public boolean applyVpnUsers(long vpnOwnerId, String userName) throws ResourceUnavailableException { + public boolean applyVpnUsers(long vpnOwnerId, String userName, boolean forRemove) throws ResourceUnavailableException { Account caller = CallContext.current().getCallingAccount(); Account owner = _accountDao.findById(vpnOwnerId); _accountMgr.checkAccess(caller, null, true, owner); s_logger.debug(String.format("Applying VPN users for %s.", owner.toString())); - List vpns = _remoteAccessVpnDao.findByAccount(vpnOwnerId); + List vpns = getValidRemoteAccessVpnForAccount(vpnOwnerId); if (CollectionUtils.isEmpty(vpns)) { - s_logger.debug(String.format("Unable to add VPN user due to there are no remote access VPNs configured on %s to apply VPN user.", owner.toString())); - return false; + if (forRemove) { + return removeVpnUserWithoutRemoteAccessVpn(vpnOwnerId, userName); + } + s_logger.warn(String.format("Unable to apply VPN user due to there are no remote access VPNs configured on %s to apply VPN user.", owner.toString())); + return true; } RemoteAccessVpnVO vpnTemp = null; @@ -597,6 +631,12 @@ public class RemoteAccessVpnManagerImpl extends ManagerBase implements RemoteAcc return success; } + @DB + @Override + public boolean applyVpnUsers(long vpnOwnerId, String userName) throws ResourceUnavailableException { + return applyVpnUsers(vpnOwnerId, userName, false); + } + @Override public Pair, Integer> searchForVpnUsers(ListVpnUsersCmd cmd) { String username = cmd.getUsername();