This is an automated email from the ASF dual-hosted git repository. harikrishna pushed a commit to branch 2FA in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 6b9fcde965d2221ced87a2d6dd250189c8a36414 Author: Harikrishna Patnala <[email protected]> AuthorDate: Mon Nov 28 02:44:01 2022 +0530 Added UnitTests --- .../UserTwoFactorAuthenticationSetupResponse.java | 4 + .../main/java/com/cloud/user/AccountManager.java | 21 ---- .../java/com/cloud/user/AccountManagerImpl.java | 27 ++++- .../com/cloud/user/AccountManagerImplTest.java | 110 +++++++++++++++++++++ .../com/cloud/user/AccountManagetImplTestBase.java | 2 + 5 files changed, 140 insertions(+), 24 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/response/UserTwoFactorAuthenticationSetupResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/UserTwoFactorAuthenticationSetupResponse.java index 10f64902e37..35beefde403 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/UserTwoFactorAuthenticationSetupResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/UserTwoFactorAuthenticationSetupResponse.java @@ -56,4 +56,8 @@ public class UserTwoFactorAuthenticationSetupResponse extends BaseResponse { public void setSecretCode(String secretCode) { this.secretCode = secretCode; } + + public String getSecretCode() { + return secretCode; + } } diff --git a/server/src/main/java/com/cloud/user/AccountManager.java b/server/src/main/java/com/cloud/user/AccountManager.java index 7990a826a97..c95047a6c42 100644 --- a/server/src/main/java/com/cloud/user/AccountManager.java +++ b/server/src/main/java/com/cloud/user/AccountManager.java @@ -190,27 +190,6 @@ public interface AccountManager extends AccountService, Configurable { ConfigKey<Boolean> UseSecretKeyInResponse = new ConfigKey<Boolean>("Advanced", Boolean.class, "use.secret.key.in.response", "false", "This parameter allows the users to enable or disable of showing secret key as a part of response for various APIs. By default it is set to false.", true); - ConfigKey<Boolean> enableUserTwoFactorAuthentication = new ConfigKey<Boolean>("Advanced", - Boolean.class, - "enable.user.two.factor.authentication", - "false", - "Determines whether two factor authentication is enabled or not. This can be configured at domain level also", - false, - ConfigKey.Scope.Domain); - - ConfigKey<Boolean> mandateUserTwoFactorAuthentication = new ConfigKey<Boolean>("Advanced", - Boolean.class, - "mandate.user.two.factor.authentication", - "false", - "Determines whether to make the two factor authentication mandatory or not. This can be configured at domain level also", - false, - ConfigKey.Scope.Domain); - - ConfigKey<String> userTwoFactorAuthenticationProviderPlugin = new ConfigKey<>("Advanced", String.class, - "user.two.factor.authentication.provider.plugin", - "GOOGLE", - "The user two factor authentication provider plugin. Eg. google, staticpin", true, ConfigKey.Scope.Domain); - boolean moveUser(long id, Long domainId, Account newAccount); UserTwoFactorAuthenticator getUserTwoFactorAuthenticator(final Long domainId, final Long userAccountId); diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index 1ab05cfaa00..85d3e470f08 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -322,10 +322,31 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M private int _cleanupInterval; private List<String> apiNameList; - private static Map<String, UserTwoFactorAuthenticator> userTwoFactorAuthenticationProvidersMap = new HashMap<>(); + protected static Map<String, UserTwoFactorAuthenticator> userTwoFactorAuthenticationProvidersMap = new HashMap<>(); private List<UserTwoFactorAuthenticator> userTwoFactorAuthenticationProviders; + static ConfigKey<Boolean> enableUserTwoFactorAuthentication = new ConfigKey<Boolean>("Advanced", + Boolean.class, + "enable.user.two.factor.authentication", + "false", + "Determines whether two factor authentication is enabled or not. This can be configured at domain level also", + false, + ConfigKey.Scope.Domain); + + ConfigKey<Boolean> mandateUserTwoFactorAuthentication = new ConfigKey<Boolean>("Advanced", + Boolean.class, + "mandate.user.two.factor.authentication", + "false", + "Determines whether to make the two factor authentication mandatory or not. This can be configured at domain level also", + false, + ConfigKey.Scope.Domain); + + ConfigKey<String> userTwoFactorAuthenticationProviderPlugin = new ConfigKey<>("Advanced", String.class, + "user.two.factor.authentication.provider.plugin", + "GOOGLE", + "The user two factor authentication provider plugin. Eg. google, staticpin", true, ConfigKey.Scope.Domain); + protected AccountManagerImpl() { super(); } @@ -3215,7 +3236,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M return response; } - private UserTwoFactorAuthenticationSetupResponse enableTwoFactorAuthentication(Long userId, String providerName) { + protected UserTwoFactorAuthenticationSetupResponse enableTwoFactorAuthentication(Long userId, String providerName) { UserAccountVO userAccount = _userAccountDao.findById(userId); UserVO userVO = _userDao.findById(userId); @@ -3242,7 +3263,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M return response; } - private UserTwoFactorAuthenticationSetupResponse disableTwoFactorAuthentication(Long userId, Account caller, Account owner) { + protected UserTwoFactorAuthenticationSetupResponse disableTwoFactorAuthentication(Long userId, Account caller, Account owner) { UserVO userVO = null; if (userId != null) { userVO = validateUser(userId, caller.getDomainId()); diff --git a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java index 4d948dda1c5..706d3686a9e 100644 --- a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java +++ b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java @@ -21,11 +21,16 @@ import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Map; +import java.util.HashMap; import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.api.command.admin.user.GetUserKeysCmd; import org.apache.cloudstack.api.command.admin.user.UpdateUserCmd; +import org.apache.cloudstack.api.response.UserTwoFactorAuthenticationSetupResponse; +import org.apache.cloudstack.auth.UserTwoFactorAuthenticator; import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.framework.config.ConfigKey; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -34,6 +39,7 @@ import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; +import static org.mockito.ArgumentMatchers.nullable; import com.cloud.acl.DomainChecker; import com.cloud.domain.Domain; @@ -92,6 +98,13 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { @Mock PasswordPolicyImpl passwordPolicyMock; + @Mock + ConfigKey<Boolean> enableUserTwoFactorAuthenticationMock; + + @Before + public void setUp() throws Exception { + enableUserTwoFactorAuthenticationMock = Mockito.mock(ConfigKey.class); + } @Before public void beforeTest() { @@ -777,4 +790,101 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { accountManagerImpl.updateLoginAttemptsWhenIncorrectLoginAttemptsEnabled(userAccountVO, true, allowedAttempts); Mockito.verify(accountManagerImpl).updateLoginAttempts(Mockito.eq(accountId), Mockito.eq(allowedAttempts), Mockito.eq(true)); } + + @Test(expected = CloudRuntimeException.class) + public void testEnableUserTwoFactorAuthenticationWhenDomainlevelSettingisDisabled() { + Long userId = 1L; + + UserAccountVO userAccount = Mockito.mock(UserAccountVO.class); + UserVO userVO = Mockito.mock(UserVO.class); + + Mockito.when(userAccountDaoMock.findById(userId)).thenReturn(userAccount); + Mockito.when(userDaoMock.findById(userId)).thenReturn(userVO); + Mockito.when(userAccount.getDomainId()).thenReturn(1L); + + ConfigKey<Boolean> enableUserTwoFactorAuthentication = Mockito.mock(ConfigKey.class); + AccountManagerImpl.enableUserTwoFactorAuthentication = enableUserTwoFactorAuthentication; + + Mockito.when(enableUserTwoFactorAuthentication.valueIn(1L)).thenReturn(false); + + accountManagerImpl.enableTwoFactorAuthentication(userId, "google"); + } + + @Test(expected = InvalidParameterValueException.class) + public void testEnableUserTwoFactorAuthenticationWhenProviderNameIsNull() { + Long userId = 1L; + + UserAccountVO userAccount = Mockito.mock(UserAccountVO.class); + UserVO userVO = Mockito.mock(UserVO.class); + + Mockito.when(userAccountDaoMock.findById(userId)).thenReturn(userAccount); + Mockito.when(userDaoMock.findById(userId)).thenReturn(userVO); + Mockito.when(userAccount.getDomainId()).thenReturn(1L); + + ConfigKey<Boolean> enableUserTwoFactorAuthentication = Mockito.mock(ConfigKey.class); + AccountManagerImpl.enableUserTwoFactorAuthentication = enableUserTwoFactorAuthentication; + + Mockito.when(enableUserTwoFactorAuthentication.valueIn(1L)).thenReturn(true); + + accountManagerImpl.enableTwoFactorAuthentication(userId, null); + } + + @Test + public void testEnableUserTwoFactorAuthentication() { + Long userId = 1L; + + UserAccountVO userAccount = Mockito.mock(UserAccountVO.class); + UserVO userVO = Mockito.mock(UserVO.class); + + Mockito.when(userAccountDaoMock.findById(userId)).thenReturn(userAccount); + Mockito.when(userDaoMock.findById(userId)).thenReturn(userVO); + Mockito.when(userAccount.getDomainId()).thenReturn(1L); + + ConfigKey<Boolean> enableUserTwoFactorAuthentication = Mockito.mock(ConfigKey.class); + AccountManagerImpl.enableUserTwoFactorAuthentication = enableUserTwoFactorAuthentication; + Mockito.when(enableUserTwoFactorAuthentication.valueIn(1L)).thenReturn(true); + + UserTwoFactorAuthenticator googleProvider = Mockito.mock(UserTwoFactorAuthenticator.class); + Map<String, UserTwoFactorAuthenticator> userTwoFactorAuthenticationProvidersMap = Mockito.mock(HashMap.class); + Mockito.when(userTwoFactorAuthenticationProvidersMap.containsKey("google")).thenReturn( true); + Mockito.when(userTwoFactorAuthenticationProvidersMap.get("google")).thenReturn( googleProvider); + AccountManagerImpl.userTwoFactorAuthenticationProvidersMap = userTwoFactorAuthenticationProvidersMap; + Mockito.when(googleProvider.setup2FAKey(userAccount)).thenReturn("EUJEAEDVOURFZTE6OGWVTJZMI54QGMIL"); + Mockito.when(userDaoMock.createForUpdate()).thenReturn(userVoMock); + Mockito.when(userDaoMock.update(userId, userVoMock)).thenReturn(true); + + UserTwoFactorAuthenticationSetupResponse response = accountManagerImpl.enableTwoFactorAuthentication(userId, "google"); + + Assert.assertEquals("EUJEAEDVOURFZTE6OGWVTJZMI54QGMIL", response.getSecretCode()); + } + + @Test + public void testDisableUserTwoFactorAuthentication() { + Long userId = 1L; + + UserVO userVO = Mockito.mock(UserVO.class); + Account caller = Mockito.mock(Account.class); + + AccountVO accountMock = Mockito.mock(AccountVO.class); + Mockito.doNothing().when(accountManagerImpl).checkAccess(nullable(Account.class), Mockito.isNull(), nullable(Boolean.class), nullable(Account.class)); + + Mockito.when(caller.getDomainId()).thenReturn(1L); + Mockito.when(userDaoMock.findById(userId)).thenReturn(userVO); + Mockito.when(userVO.getAccountId()).thenReturn(1L); + Mockito.when(_accountDao.findById(1L)).thenReturn(accountMock); + Mockito.when(accountMock.getDomainId()).thenReturn(1L); + Mockito.when(_accountService.getActiveAccountById(1L)).thenReturn(caller); + + userVoMock.setKeyFor2fa("EUJEAEDVOURFZTE6OGWVTJZMI54QGMIL"); + userVoMock.setUser2faProvider("google"); + userVoMock.setTwoFactorAuthenticationEnabled(true); + + Mockito.when(userDaoMock.createForUpdate()).thenReturn(userVoMock); + + UserTwoFactorAuthenticationSetupResponse response = accountManagerImpl.disableTwoFactorAuthentication(userId, caller, caller); + + Assert.assertNull(response.getSecretCode()); + Assert.assertNull(userVoMock.getKeyFor2fa()); + Assert.assertNull(userVoMock.getUser2faProvider()); + } } diff --git a/server/src/test/java/com/cloud/user/AccountManagetImplTestBase.java b/server/src/test/java/com/cloud/user/AccountManagetImplTestBase.java index d0a70faea5e..764c780da38 100644 --- a/server/src/test/java/com/cloud/user/AccountManagetImplTestBase.java +++ b/server/src/test/java/com/cloud/user/AccountManagetImplTestBase.java @@ -198,6 +198,8 @@ public class AccountManagetImplTestBase { AccountManagerImpl accountManagerImpl; @Mock UsageEventDao _usageEventDao; + @Mock + AccountService _accountService; @Before public void setup() {
