server: reset 2fa user configuration on incomplete setup (#10247)

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
This commit is contained in:
Abhishek Kumar 2025-01-30 20:21:04 +05:30 committed by GitHub
parent b9890875cc
commit b93589b5bd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 95 additions and 1 deletions

View File

@ -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"));
}
}

View File

@ -516,4 +516,9 @@ public class MockAccountManager extends ManagerBase implements AccountManager {
public ConfigKey<?>[] getConfigKeys() {
return null;
}
@Override
public UserAccount clearUserTwoFactorAuthenticationInSetupStateOnLogin(UserAccount user) {
return null;
}
}

View File

@ -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;

View File

@ -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);
}

View File

@ -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;
});
}
}

View File

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

View File

@ -484,4 +484,8 @@ public class MockAccountManagerImpl extends ManagerBase implements Manager, Acco
return null;
}
@Override
public UserAccount clearUserTwoFactorAuthenticationInSetupStateOnLogin(UserAccount user) {
return null;
}
}