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 c18ca53f7abe..a9d4ca9a2b98 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 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 Map getDetails() { public void setDetails(Map 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 a3fac2c8be92..a63c5b68e579 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 String getConfigComponentName() { 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 d68f42470d55..c78f8e68c2bb 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 ResponseObject loginUser(final HttpSession session, final String username 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 ResponseObject loginUser(final HttpSession session, final String username 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 c95047a6c42b..1d7611d5b542 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 @@ void buildACLViewSearchCriteria(SearchCriteria 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); + } diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index 1e727036d565..7d22a1143313 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 UserTwoFactorAuthenticator getUserTwoFactorAuthenticator(final String nam return userTwoFactorAuthenticationProvidersMap.get(name.toLowerCase()); } + @Override + public UserAccount clearUserTwoFactorAuthenticationInSetupStateOnLogin(UserAccount user) { + return Transaction.execute((TransactionCallback) 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 0e8e1df0f3a0..e68de194f019 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.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 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 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 f85589924409..96b73cc63dd3 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 ConfigKey[] getConfigKeys() { return null; } + @Override + public UserAccount clearUserTwoFactorAuthenticationInSetupStateOnLogin(UserAccount user) { + return null; + } }