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
7 changes: 7 additions & 0 deletions engine/schema/src/main/java/com/cloud/user/UserAccountVO.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@

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
Expand Down Expand Up @@ -368,4 +370,9 @@
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"));
}

Check warning on line 377 in engine/schema/src/main/java/com/cloud/user/UserAccountVO.java

View check run for this annotation

Codecov / codecov/patch

engine/schema/src/main/java/com/cloud/user/UserAccountVO.java#L375-L377

Added lines #L375 - L377 were not covered by tests
}
Original file line number Diff line number Diff line change
Expand Up @@ -516,4 +516,9 @@ public String getConfigComponentName() {
public ConfigKey<?>[] getConfigKeys() {
return null;
}

@Override
public UserAccount clearUserTwoFactorAuthenticationInSetupStateOnLogin(UserAccount user) {
return null;
}
}
3 changes: 2 additions & 1 deletion server/src/main/java/com/cloud/api/ApiServer.java
Original file line number Diff line number Diff line change
Expand Up @@ -1159,7 +1159,7 @@
domainId = userDomain.getId();
}

final UserAccount userAcct = accountMgr.authenticateUser(username, password, domainId, loginIpAddress, requestParameters);
UserAccount userAcct = accountMgr.authenticateUser(username, password, domainId, loginIpAddress, requestParameters);

Check warning on line 1162 in server/src/main/java/com/cloud/api/ApiServer.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/api/ApiServer.java#L1162

Added line #L1162 was not covered by tests
if (userAcct != null) {
final String timezone = userAcct.getTimezone();
float offsetInHrs = 0f;
Expand Down Expand Up @@ -1204,6 +1204,7 @@
session.setAttribute("timezoneoffset", Float.valueOf(offsetInHrs).toString());
}

userAcct = accountMgr.clearUserTwoFactorAuthenticationInSetupStateOnLogin(userAcct);

Check warning on line 1207 in server/src/main/java/com/cloud/api/ApiServer.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/api/ApiServer.java#L1207

Added line #L1207 was not covered by tests
boolean is2faEnabled = false;
if (userAcct.isUser2faEnabled() || (Boolean.TRUE.equals(AccountManagerImpl.enableUserTwoFactorAuthentication.valueIn(userAcct.getDomainId())) && Boolean.TRUE.equals(AccountManagerImpl.mandateUserTwoFactorAuthentication.valueIn(userAcct.getDomainId())))) {
is2faEnabled = true;
Expand Down
3 changes: 3 additions & 0 deletions server/src/main/java/com/cloud/user/AccountManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,9 @@ void buildACLViewSearchCriteria(SearchCriteria<? extends ControlledViewEntity> s
UserTwoFactorAuthenticator getUserTwoFactorAuthenticator(final Long domainId, final Long userAccountId);

void verifyUsingTwoFactorAuthenticationCode(String code, Long domainId, Long userAccountId);

UserTwoFactorAuthenticationSetupResponse setupUserTwoFactorAuthentication(SetupUserTwoFactorAuthenticationCmd cmd);

UserAccount clearUserTwoFactorAuthenticationInSetupStateOnLogin(UserAccount user);

}
22 changes: 22 additions & 0 deletions server/src/main/java/com/cloud/user/AccountManagerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -3475,4 +3475,26 @@ public UserTwoFactorAuthenticator getUserTwoFactorAuthenticator(final String nam
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;
});
}
}
52 changes: 52 additions & 0 deletions server/src/test/java/com/cloud/user/AccountManagerImplTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,12 @@
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;
Expand Down Expand Up @@ -1218,4 +1220,54 @@ public void checkIfAccountManagesProjectsTestThrowExceptionWhenTheAccountIsAProj
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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -484,4 +484,8 @@ public ConfigKey<?>[] getConfigKeys() {
return null;
}

@Override
public UserAccount clearUserTwoFactorAuthenticationInSetupStateOnLogin(UserAccount user) {
return null;
}
}
Loading