This is an automated email from the ASF dual-hosted git repository.
dahn pushed a commit to branch 4.19
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.19 by this push:
new b93589b5bdf server: reset 2fa user configuration on incomplete setup
(#10247)
b93589b5bdf is described below
commit b93589b5bdf52043127a9c898d6aa596b0245c4c
Author: Abhishek Kumar <[email protected]>
AuthorDate: Thu Jan 30 20:21:04 2025 +0530
server: reset 2fa user configuration on incomplete setup (#10247)
Signed-off-by: Abhishek Kumar <[email protected]>
---
.../main/java/com/cloud/user/UserAccountVO.java | 7 +++
.../contrail/management/MockAccountManager.java | 5 +++
server/src/main/java/com/cloud/api/ApiServer.java | 3 +-
.../main/java/com/cloud/user/AccountManager.java | 3 ++
.../java/com/cloud/user/AccountManagerImpl.java | 22 +++++++++
.../com/cloud/user/AccountManagerImplTest.java | 52 ++++++++++++++++++++++
.../com/cloud/user/MockAccountManagerImpl.java | 4 ++
7 files changed, 95 insertions(+), 1 deletion(-)
diff --git a/engine/schema/src/main/java/com/cloud/user/UserAccountVO.java
b/engine/schema/src/main/java/com/cloud/user/UserAccountVO.java
index c18ca53f7ab..a9d4ca9a2b9 100644
--- a/engine/schema/src/main/java/com/cloud/user/UserAccountVO.java
+++ b/engine/schema/src/main/java/com/cloud/user/UserAccountVO.java
@@ -35,6 +35,8 @@ import org.apache.cloudstack.api.InternalIdentity;
import com.cloud.utils.db.Encrypt;
import com.cloud.utils.db.GenericDao;
+
+import
org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
import org.apache.commons.lang3.StringUtils;
@Entity
@@ -368,4 +370,9 @@ public class UserAccountVO implements UserAccount,
InternalIdentity {
public void setDetails(Map<String, String> details) {
this.details = details;
}
+
+ @Override
+ public String toString() {
+ return String.format("User %s",
ReflectionToStringBuilderUtils.reflectOnlySelectedFields(this, "id", "name",
"uuid"));
+ }
}
diff --git
a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java
b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java
index a3fac2c8be9..a63c5b68e57 100644
---
a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java
+++
b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java
@@ -516,4 +516,9 @@ public class MockAccountManager extends ManagerBase
implements AccountManager {
public ConfigKey<?>[] getConfigKeys() {
return null;
}
+
+ @Override
+ public UserAccount
clearUserTwoFactorAuthenticationInSetupStateOnLogin(UserAccount user) {
+ return null;
+ }
}
diff --git a/server/src/main/java/com/cloud/api/ApiServer.java
b/server/src/main/java/com/cloud/api/ApiServer.java
index d68f42470d5..c78f8e68c2b 100644
--- a/server/src/main/java/com/cloud/api/ApiServer.java
+++ b/server/src/main/java/com/cloud/api/ApiServer.java
@@ -1159,7 +1159,7 @@ public class ApiServer extends ManagerBase implements
HttpRequestHandler, ApiSer
domainId = userDomain.getId();
}
- final UserAccount userAcct = accountMgr.authenticateUser(username,
password, domainId, loginIpAddress, requestParameters);
+ UserAccount userAcct = accountMgr.authenticateUser(username, password,
domainId, loginIpAddress, requestParameters);
if (userAcct != null) {
final String timezone = userAcct.getTimezone();
float offsetInHrs = 0f;
@@ -1204,6 +1204,7 @@ public class ApiServer extends ManagerBase implements
HttpRequestHandler, ApiSer
session.setAttribute("timezoneoffset",
Float.valueOf(offsetInHrs).toString());
}
+ userAcct =
accountMgr.clearUserTwoFactorAuthenticationInSetupStateOnLogin(userAcct);
boolean is2faEnabled = false;
if (userAcct.isUser2faEnabled() ||
(Boolean.TRUE.equals(AccountManagerImpl.enableUserTwoFactorAuthentication.valueIn(userAcct.getDomainId()))
&&
Boolean.TRUE.equals(AccountManagerImpl.mandateUserTwoFactorAuthentication.valueIn(userAcct.getDomainId()))))
{
is2faEnabled = true;
diff --git a/server/src/main/java/com/cloud/user/AccountManager.java
b/server/src/main/java/com/cloud/user/AccountManager.java
index c95047a6c42..1d7611d5b54 100644
--- a/server/src/main/java/com/cloud/user/AccountManager.java
+++ b/server/src/main/java/com/cloud/user/AccountManager.java
@@ -195,6 +195,9 @@ public interface AccountManager extends AccountService,
Configurable {
UserTwoFactorAuthenticator getUserTwoFactorAuthenticator(final Long
domainId, final Long userAccountId);
void verifyUsingTwoFactorAuthenticationCode(String code, Long domainId,
Long userAccountId);
+
UserTwoFactorAuthenticationSetupResponse
setupUserTwoFactorAuthentication(SetupUserTwoFactorAuthenticationCmd cmd);
+ UserAccount
clearUserTwoFactorAuthenticationInSetupStateOnLogin(UserAccount user);
+
}
diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java
b/server/src/main/java/com/cloud/user/AccountManagerImpl.java
index 1e727036d56..7d22a114331 100644
--- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java
+++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java
@@ -3475,4 +3475,26 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
return userTwoFactorAuthenticationProvidersMap.get(name.toLowerCase());
}
+ @Override
+ public UserAccount
clearUserTwoFactorAuthenticationInSetupStateOnLogin(UserAccount user) {
+ return Transaction.execute((TransactionCallback<UserAccount>) status
-> {
+ if (!user.isUser2faEnabled() &&
StringUtils.isBlank(user.getUser2faProvider())) {
+ return user;
+ }
+ UserDetailVO userDetailVO =
_userDetailsDao.findDetail(user.getId(), UserDetailVO.Setup2FADetail);
+ if (userDetailVO != null &&
UserAccountVO.Setup2FAstatus.VERIFIED.name().equals(userDetailVO.getValue())) {
+ return user;
+ }
+ s_logger.info(String.format("Clearing 2FA configurations for %s as
it is still in setup on a new login request", user));
+ if (userDetailVO != null) {
+ _userDetailsDao.remove(userDetailVO.getId());
+ }
+ UserAccountVO userAccountVO =
_userAccountDao.findById(user.getId());
+ userAccountVO.setUser2faEnabled(false);
+ userAccountVO.setUser2faProvider(null);
+ userAccountVO.setKeyFor2fa(null);
+ _userAccountDao.update(user.getId(), userAccountVO);
+ return userAccountVO;
+ });
+ }
}
diff --git a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java
b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java
index 0e8e1df0f3a..e68de194f01 100644
--- a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java
+++ b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java
@@ -39,10 +39,12 @@ import
org.apache.cloudstack.auth.UserAuthenticator.ActionOnFailedAuthentication
import org.apache.cloudstack.auth.UserTwoFactorAuthenticator;
import org.apache.cloudstack.context.CallContext;
import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.resourcedetail.UserDetailVO;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
import org.mockito.InOrder;
import org.mockito.Mock;
import org.mockito.Mockito;
@@ -1218,4 +1220,54 @@ public class AccountManagerImplTest extends
AccountManagetImplTestBase {
Mockito.when(_projectAccountDao.listAdministratedProjectIds(accountId)).thenReturn(managedProjectIds);
accountManagerImpl.checkIfAccountManagesProjects(accountId);
}
+
+ @Test
+ public void testClearUser2FA_When2FADisabled_NoChanges() {
+ UserAccount user = Mockito.mock(UserAccount.class);
+ Mockito.when(user.isUser2faEnabled()).thenReturn(false);
+ Mockito.when(user.getUser2faProvider()).thenReturn(null);
+ UserAccount result =
accountManagerImpl.clearUserTwoFactorAuthenticationInSetupStateOnLogin(user);
+ Assert.assertSame(user, result);
+ Mockito.verifyNoInteractions(userDetailsDaoMock, userAccountDaoMock);
+ }
+
+ @Test
+ public void testClearUser2FA_When2FAInVerifiedState_NoChanges() {
+ UserAccount user = Mockito.mock(UserAccount.class);
+ Mockito.when(user.getId()).thenReturn(1L);
+ Mockito.when(user.isUser2faEnabled()).thenReturn(true);
+ UserDetailVO userDetail = new UserDetailVO();
+ userDetail.setValue(UserAccountVO.Setup2FAstatus.VERIFIED.name());
+ Mockito.when(userDetailsDaoMock.findDetail(1L,
UserDetailVO.Setup2FADetail)).thenReturn(userDetail);
+ UserAccount result =
accountManagerImpl.clearUserTwoFactorAuthenticationInSetupStateOnLogin(user);
+ Assert.assertSame(user, result);
+ Mockito.verify(userDetailsDaoMock).findDetail(1L,
UserDetailVO.Setup2FADetail);
+ Mockito.verifyNoMoreInteractions(userDetailsDaoMock,
userAccountDaoMock);
+ }
+
+ @Test
+ public void testClearUser2FA_When2FAInSetupState_Disable2FA() {
+ UserAccount user = Mockito.mock(UserAccount.class);
+ Mockito.when(user.getId()).thenReturn(1L);
+ Mockito.when(user.isUser2faEnabled()).thenReturn(true);
+ UserDetailVO userDetail = new UserDetailVO();
+ userDetail.setValue(UserAccountVO.Setup2FAstatus.ENABLED.name());
+ UserAccountVO userAccountVO = new UserAccountVO();
+ userAccountVO.setId(1L);
+ Mockito.when(userDetailsDaoMock.findDetail(1L,
UserDetailVO.Setup2FADetail)).thenReturn(userDetail);
+
Mockito.when(userAccountDaoMock.findById(1L)).thenReturn(userAccountVO);
+ UserAccount result =
accountManagerImpl.clearUserTwoFactorAuthenticationInSetupStateOnLogin(user);
+ Assert.assertNotNull(result);
+ Assert.assertFalse(result.isUser2faEnabled());
+ Assert.assertNull(result.getUser2faProvider());
+ Mockito.verify(userDetailsDaoMock).findDetail(1L,
UserDetailVO.Setup2FADetail);
+ Mockito.verify(userDetailsDaoMock).remove(Mockito.anyLong());
+ Mockito.verify(userAccountDaoMock).findById(1L);
+ ArgumentCaptor<UserAccountVO> captor =
ArgumentCaptor.forClass(UserAccountVO.class);
+ Mockito.verify(userAccountDaoMock).update(Mockito.eq(1L),
captor.capture());
+ UserAccountVO updatedUser = captor.getValue();
+ Assert.assertFalse(updatedUser.isUser2faEnabled());
+ Assert.assertNull(updatedUser.getUser2faProvider());
+ Assert.assertNull(updatedUser.getKeyFor2fa());
+ }
}
diff --git a/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java
b/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java
index f8558992440..96b73cc63dd 100644
--- a/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java
+++ b/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java
@@ -484,4 +484,8 @@ public class MockAccountManagerImpl extends ManagerBase
implements Manager, Acco
return null;
}
+ @Override
+ public UserAccount
clearUserTwoFactorAuthenticationInSetupStateOnLogin(UserAccount user) {
+ return null;
+ }
}
