diff --git a/application/src/main/java/org/thingsboard/server/controller/TwoFactorAuthController.java b/application/src/main/java/org/thingsboard/server/controller/TwoFactorAuthController.java index b95e1d6099..b93df6e751 100644 --- a/application/src/main/java/org/thingsboard/server/controller/TwoFactorAuthController.java +++ b/application/src/main/java/org/thingsboard/server/controller/TwoFactorAuthController.java @@ -36,13 +36,7 @@ import org.thingsboard.server.service.security.system.SystemSecurityService; import javax.servlet.http.HttpServletRequest; /* - * TODO [viacheslav]: - * - Swagger documentation - * - * TODO [viacheslav] (later): - * - 2FA entries should be secured against code injection by code validation - * - ability to force users to use 2FA (maybe on log in, do not give them token pair but to give temporary - * token to configure 2FA account config); also will need to make users configure 2FA during activation and password setup... + * FIXME [viacheslav]: Swagger documentation * */ @RestController @RequestMapping("/api/auth/2fa") diff --git a/application/src/test/java/org/thingsboard/server/controller/TwoFactorAuthConfigTest.java b/application/src/test/java/org/thingsboard/server/controller/TwoFactorAuthConfigTest.java index 01e2bc496e..38ff2e0702 100644 --- a/application/src/test/java/org/thingsboard/server/controller/TwoFactorAuthConfigTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/TwoFactorAuthConfigTest.java @@ -29,7 +29,6 @@ import org.springframework.web.util.UriComponents; import org.springframework.web.util.UriComponentsBuilder; import org.thingsboard.rule.engine.api.SmsService; import org.thingsboard.server.common.data.CacheConstants; -import org.thingsboard.server.common.data.exception.ThingsboardException; import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.service.security.auth.mfa.config.TwoFactorAuthConfigManager; import org.thingsboard.server.service.security.auth.mfa.config.TwoFactorAuthSettings; @@ -100,7 +99,7 @@ public abstract class TwoFactorAuthConfigTest extends AbstractControllerTest { twoFaSettings.setVerificationCodeSendRateLimit("1:60"); twoFaSettings.setVerificationCodeCheckRateLimit("3:900"); twoFaSettings.setMaxVerificationFailuresBeforeUserLockout(10); - twoFaSettings.setTotalAllowedTimeForVerification(60); + twoFaSettings.setTotalAllowedTimeForVerification(3600); doPost("/api/2fa/settings", twoFaSettings).andExpect(status().isOk()); @@ -497,9 +496,10 @@ public abstract class TwoFactorAuthConfigTest extends AbstractControllerTest { } @Test - public void testIsTwoFaEnabled() throws ThingsboardException { + public void testIsTwoFaEnabled() throws Exception { + configureSmsTwoFaProvider("${verificationCode}"); SmsTwoFactorAuthAccountConfig accountConfig = new SmsTwoFactorAuthAccountConfig(); - accountConfig.setPhoneNumber("+380505050"); + accountConfig.setPhoneNumber("+38050505050"); twoFactorAuthConfigManager.saveTwoFaAccountConfig(tenantId, tenantAdminUserId, accountConfig); assertThat(twoFactorAuthConfigManager.isTwoFaEnabled(tenantId, tenantAdminUserId)).isTrue(); @@ -507,12 +507,14 @@ public abstract class TwoFactorAuthConfigTest extends AbstractControllerTest { @Test public void testDeleteTwoFaAccountConfig() throws Exception { + configureSmsTwoFaProvider("${verificationCode}"); SmsTwoFactorAuthAccountConfig accountConfig = new SmsTwoFactorAuthAccountConfig(); - accountConfig.setPhoneNumber("+380505050"); - twoFactorAuthConfigManager.saveTwoFaAccountConfig(tenantId, tenantAdminUserId, accountConfig); + accountConfig.setPhoneNumber("+38050505050"); loginTenantAdmin(); + twoFactorAuthConfigManager.saveTwoFaAccountConfig(tenantId, tenantAdminUserId, accountConfig); + TwoFactorAuthAccountConfig savedAccountConfig = readResponse(doGet("/api/2fa/account/config").andExpect(status().isOk()), TwoFactorAuthAccountConfig.class); assertThat(savedAccountConfig).isEqualTo(accountConfig); diff --git a/application/src/test/java/org/thingsboard/server/controller/TwoFactorAuthTest.java b/application/src/test/java/org/thingsboard/server/controller/TwoFactorAuthTest.java index 4a9888ada1..09d1208df3 100644 --- a/application/src/test/java/org/thingsboard/server/controller/TwoFactorAuthTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/TwoFactorAuthTest.java @@ -15,15 +15,372 @@ */ package org.thingsboard.server.controller; -/* -* TODO [viacheslav] -* check validation of the verification code -* test rate limits -* test code expiration -* test pre-verification token lifetime -* test user blocking -* test log login action, lastLoginTs, and authentication details -* */ +import com.fasterxml.jackson.databind.JsonNode; +import org.apache.commons.lang3.RandomStringUtils; +import org.apache.commons.lang3.StringUtils; +import org.jboss.aerogear.security.otp.Totp; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.mockito.ArgumentCaptor; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.thingsboard.rule.engine.api.SmsService; +import org.thingsboard.server.common.data.User; +import org.thingsboard.server.common.data.audit.ActionStatus; +import org.thingsboard.server.common.data.audit.ActionType; +import org.thingsboard.server.common.data.audit.AuditLog; +import org.thingsboard.server.common.data.exception.ThingsboardException; +import org.thingsboard.server.common.data.id.TenantId; +import org.thingsboard.server.common.data.page.PageLink; +import org.thingsboard.server.common.data.page.SortOrder; +import org.thingsboard.server.common.data.page.TimePageLink; +import org.thingsboard.server.common.data.security.Authority; +import org.thingsboard.server.dao.audit.AuditLogService; +import org.thingsboard.server.dao.user.UserService; +import org.thingsboard.server.service.security.auth.mfa.TwoFactorAuthService; +import org.thingsboard.server.service.security.auth.mfa.config.TwoFactorAuthConfigManager; +import org.thingsboard.server.service.security.auth.mfa.config.TwoFactorAuthSettings; +import org.thingsboard.server.service.security.auth.mfa.config.account.SmsTwoFactorAuthAccountConfig; +import org.thingsboard.server.service.security.auth.mfa.config.account.TotpTwoFactorAuthAccountConfig; +import org.thingsboard.server.service.security.auth.mfa.config.provider.SmsTwoFactorAuthProviderConfig; +import org.thingsboard.server.service.security.auth.mfa.config.provider.TotpTwoFactorAuthProviderConfig; +import org.thingsboard.server.service.security.auth.mfa.config.provider.TwoFactorAuthProviderConfig; +import org.thingsboard.server.service.security.auth.mfa.provider.TwoFactorAuthProviderType; +import org.thingsboard.server.service.security.auth.rest.LoginRequest; +import org.thingsboard.server.service.security.model.JwtTokenPair; + +import java.time.Duration; +import java.util.Arrays; +import java.util.List; +import java.util.concurrent.TimeUnit; +import java.util.function.Consumer; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.verify; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + public abstract class TwoFactorAuthTest extends AbstractControllerTest { + @Autowired + private TwoFactorAuthConfigManager twoFactorAuthConfigManager; + @Autowired + private TwoFactorAuthService twoFactorAuthService; + @MockBean + private SmsService smsService; + @Autowired + private AuditLogService auditLogService; + @Autowired + private UserService userService; + + private User user; + private String username; + private String password; + + @Before + public void beforeEach() throws Exception { + username = "mfa@tb.io"; + password = "psswrd"; + + user = new User(); + user.setAuthority(Authority.TENANT_ADMIN); + user.setEmail(username); + user.setTenantId(tenantId); + + loginSysAdmin(); + user = createUser(user, password); + } + + @After + public void afterEach() { + twoFactorAuthConfigManager.deleteTwoFaSettings(tenantId); + twoFactorAuthConfigManager.deleteTwoFaSettings(TenantId.SYS_TENANT_ID); + } + + @Test + public void testTwoFa_totp() throws Exception { + TotpTwoFactorAuthAccountConfig totpTwoFaAccountConfig = configureTotpTwoFa(); + + logInWithPreVerificationToken(); + + doPost("/api/auth/2fa/verification/send") + .andExpect(status().isOk()); + + String correctVerificationCode = getCorrectTotp(totpTwoFaAccountConfig); + + JsonNode tokenPair = readResponse(doPost("/api/auth/2fa/verification/check?verificationCode=" + correctVerificationCode) + .andExpect(status().isOk()), JsonNode.class); + validateAndSetJwtToken(tokenPair, username); + + User currentUser = readResponse(doGet("/api/auth/user") + .andExpect(status().isOk()), User.class); + assertThat(currentUser.getId()).isEqualTo(user.getId()); + } + + @Test + public void testTwoFa_sms() throws Exception { + configureSmsTwoFa(); + + logInWithPreVerificationToken(); + + doPost("/api/auth/2fa/verification/send") + .andExpect(status().isOk()); + + ArgumentCaptor verificationCodeCaptor = ArgumentCaptor.forClass(String.class); + verify(smsService).sendSms(eq(tenantId), any(), any(), verificationCodeCaptor.capture()); + String correctVerificationCode = verificationCodeCaptor.getValue(); + + JsonNode tokenPair = readResponse(doPost("/api/auth/2fa/verification/check?verificationCode=" + correctVerificationCode) + .andExpect(status().isOk()), JsonNode.class); + validateAndSetJwtToken(tokenPair, username); + + User currentUser = readResponse(doGet("/api/auth/user") + .andExpect(status().isOk()), User.class); + assertThat(currentUser.getId()).isEqualTo(user.getId()); + } + + @Test + public void testTwoFaPreVerificationTokenLifetime() throws Exception { + configureTotpTwoFa(twoFaSettings -> { + twoFaSettings.setTotalAllowedTimeForVerification(5); + }); + + logInWithPreVerificationToken(); + + await("expiration of the pre-verification token") + .atLeast(Duration.ofSeconds(3).plusMillis(500)) + .atMost(Duration.ofSeconds(6)) + .untilAsserted(() -> { + doPost("/api/auth/2fa/verification/send") + .andExpect(status().isUnauthorized()); + }); + } + + @Test + public void testCheckVerificationCode_userBlocked() throws Exception { + configureTotpTwoFa(twoFaSettings -> { + twoFaSettings.setMaxVerificationFailuresBeforeUserLockout(10); + }); + + logInWithPreVerificationToken(); + + Stream.generate(() -> RandomStringUtils.randomNumeric(6)) + .limit(9) + .forEach(incorrectVerificationCode -> { + try { + String errorMessage = getErrorMessage(doPost("/api/auth/2fa/verification/check?verificationCode=" + incorrectVerificationCode) + .andExpect(status().isBadRequest())); + assertThat(errorMessage).containsIgnoringCase("verification code is incorrect"); + } catch (Exception e) { + fail(); + } + }); + + String errorMessage = getErrorMessage(doPost("/api/auth/2fa/verification/check?verificationCode=" + RandomStringUtils.randomNumeric(6)) + .andExpect(status().isUnauthorized())); + assertThat(errorMessage).containsIgnoringCase("account was locked due to exceeded 2fa verification attempts"); + + errorMessage = getErrorMessage(doPost("/api/auth/2fa/verification/check?verificationCode=" + RandomStringUtils.randomNumeric(6)) + .andExpect(status().isUnauthorized())); + assertThat(errorMessage).containsIgnoringCase("user is disabled"); + } + + @Test + public void testSendVerificationCode_rateLimit() throws Exception { + configureTotpTwoFa(twoFaSettings -> { + twoFaSettings.setVerificationCodeSendRateLimit("3:10"); + }); + + logInWithPreVerificationToken(); + + for (int i = 0; i < 3; i++) { + doPost("/api/auth/2fa/verification/send") + .andExpect(status().isOk()); + } + + String rateLimitExceededError = getErrorMessage(doPost("/api/auth/2fa/verification/send") + .andExpect(status().isTooManyRequests())); + assertThat(rateLimitExceededError).containsIgnoringCase("too many verification code sending requests"); + + await("verification code sending rate limit resetting") + .atLeast(Duration.ofSeconds(8)) + .atMost(Duration.ofSeconds(12)) + .untilAsserted(() -> { + doPost("/api/auth/2fa/verification/send") + .andExpect(status().isOk()); + }); + } + + @Test + public void testCheckVerificationCode_rateLimit() throws Exception { + TotpTwoFactorAuthAccountConfig totpTwoFaAccountConfig = configureTotpTwoFa(twoFaSettings -> { + twoFaSettings.setVerificationCodeCheckRateLimit("3:10"); + }); + + logInWithPreVerificationToken(); + + for (int i = 0; i < 3; i++) { + String incorrectVerificationCodeError = getErrorMessage(doPost("/api/auth/2fa/verification/check?verificationCode=incorrect") + .andExpect(status().isBadRequest())); + assertThat(incorrectVerificationCodeError).containsIgnoringCase("verification code is incorrect"); + } + + String rateLimitExceededError = getErrorMessage(doPost("/api/auth/2fa/verification/check?verificationCode=incorrect") + .andExpect(status().isTooManyRequests())); + assertThat(rateLimitExceededError).containsIgnoringCase("too many verification code checking requests"); + + await("verification code checking rate limit resetting") + .atLeast(Duration.ofSeconds(8)) + .atMost(Duration.ofSeconds(12)) + .untilAsserted(() -> { + String incorrectVerificationCodeError = getErrorMessage(doPost("/api/auth/2fa/verification/check?verificationCode=incorrect") + .andExpect(status().isBadRequest())); + assertThat(incorrectVerificationCodeError).containsIgnoringCase("verification code is incorrect"); + }); + + doPost("/api/auth/2fa/verification/check?verificationCode=" + getCorrectTotp(totpTwoFaAccountConfig)) + .andExpect(status().isOk()); + } + + @Test + public void testCheckVerificationCode_invalidVerificationCode() throws Exception { + configureTotpTwoFa(); + logInWithPreVerificationToken(); + + for (String invalidVerificationCode : new String[]{"1234567", "ab1212", "12311 ", "oewkriwejqf"}) { + String errorMessage = getErrorMessage(doPost("/api/auth/2fa/verification/check?verificationCode=" + invalidVerificationCode) + .andExpect(status().isBadRequest())); + assertThat(errorMessage).containsIgnoringCase("verification code is incorrect"); + } + } + + @Test + public void testCheckVerificationCode_codeExpiration() throws Exception { + configureSmsTwoFa(smsTwoFaProviderConfig -> { + smsTwoFaProviderConfig.setVerificationCodeLifetime(10); + }); + + logInWithPreVerificationToken(); + + ArgumentCaptor verificationCodeCaptor = ArgumentCaptor.forClass(String.class); + doPost("/api/auth/2fa/verification/send").andExpect(status().isOk()); + verify(smsService).sendSms(eq(tenantId), any(), any(), verificationCodeCaptor.capture()); + + String correctVerificationCode = verificationCodeCaptor.getValue(); + + await("verification code expiration") + .pollDelay(10, TimeUnit.SECONDS) + .atLeast(10, TimeUnit.SECONDS) + .atMost(12, TimeUnit.SECONDS) + .untilAsserted(() -> { + String incorrectVerificationCodeError = getErrorMessage(doPost("/api/auth/2fa/verification/check?verificationCode=" + correctVerificationCode) + .andExpect(status().isBadRequest())); + assertThat(incorrectVerificationCodeError).containsIgnoringCase("verification code is incorrect"); + }); + } + + @Test + public void testTwoFa_logLoginAction() throws Exception { + TotpTwoFactorAuthAccountConfig totpTwoFaAccountConfig = configureTotpTwoFa(); + + logInWithPreVerificationToken(); + await("async audit log saving").during(1, TimeUnit.SECONDS); + assertThat(getLogInAuditLogs()).isEmpty(); + assertThat(userService.findUserById(tenantId, user.getId()).getAdditionalInfo() + .get("lastLoginTs")).isNull(); + + doPost("/api/auth/2fa/verification/check?verificationCode=incorrect") + .andExpect(status().isBadRequest()); + + await("async audit log saving").atMost(1, TimeUnit.SECONDS) + .until(() -> getLogInAuditLogs().size() == 1); + assertThat(getLogInAuditLogs().get(0)).satisfies(failedLogInAuditLog -> { + assertThat(failedLogInAuditLog.getActionStatus()).isEqualTo(ActionStatus.FAILURE); + assertThat(failedLogInAuditLog.getActionFailureDetails()).containsIgnoringCase("verification code is incorrect"); + assertThat(failedLogInAuditLog.getUserName()).isEqualTo(username); + }); + + doPost("/api/auth/2fa/verification/check?verificationCode=" + getCorrectTotp(totpTwoFaAccountConfig)) + .andExpect(status().isOk()); + await("async audit log saving").atMost(1, TimeUnit.SECONDS) + .until(() -> getLogInAuditLogs().size() == 2); + assertThat(getLogInAuditLogs().get(0)).satisfies(successfulLogInAuditLog -> { + assertThat(successfulLogInAuditLog.getActionStatus()).isEqualTo(ActionStatus.SUCCESS); + assertThat(successfulLogInAuditLog.getUserName()).isEqualTo(username); + }); + assertThat(userService.findUserById(tenantId, user.getId()).getAdditionalInfo() + .get("lastLoginTs").asLong()) + .isGreaterThan(System.currentTimeMillis() - TimeUnit.SECONDS.toMillis(3)); + } + + private List getLogInAuditLogs() { + return auditLogService.findAuditLogsByTenantIdAndUserId(tenantId, user.getId(), List.of(ActionType.LOGIN), + new TimePageLink(new PageLink(10, 0, null, new SortOrder("createdTime", SortOrder.Direction.DESC)), 0L, System.currentTimeMillis())).getData(); + } + + @Test + public void testAuthWithoutTwoFaAccountConfig() throws ThingsboardException { + configureTotpTwoFa(); + twoFactorAuthConfigManager.deleteTwoFaAccountConfig(tenantId, user.getId()); + + assertDoesNotThrow(() -> { + login(username, password); + }); + } + + private void logInWithPreVerificationToken() throws Exception { + LoginRequest loginRequest = new LoginRequest(username, password); + + JwtTokenPair response = readResponse(doPost("/api/auth/login", loginRequest).andExpect(status().isOk()), JwtTokenPair.class); + assertThat(response.getToken()).isNotNull(); + assertThat(response.getRefreshToken()).isNull(); + assertThat(response.getScope()).isEqualTo(Authority.PRE_VERIFICATION_TOKEN); + + this.token = response.getToken(); + } + + private TotpTwoFactorAuthAccountConfig configureTotpTwoFa(Consumer... customizer) throws ThingsboardException { + TotpTwoFactorAuthProviderConfig totpTwoFaProviderConfig = new TotpTwoFactorAuthProviderConfig(); + totpTwoFaProviderConfig.setIssuerName("tb"); + + TwoFactorAuthSettings twoFaSettings = new TwoFactorAuthSettings(); + twoFaSettings.setUseSystemTwoFactorAuthSettings(false); + twoFaSettings.setProviders(Arrays.stream(new TwoFactorAuthProviderConfig[]{totpTwoFaProviderConfig}).collect(Collectors.toList())); + Arrays.stream(customizer).forEach(c -> c.accept(twoFaSettings)); + twoFactorAuthConfigManager.saveTwoFaSettings(tenantId, twoFaSettings); + + TotpTwoFactorAuthAccountConfig totpTwoFaAccountConfig = (TotpTwoFactorAuthAccountConfig) twoFactorAuthService.generateNewAccountConfig(user, TwoFactorAuthProviderType.TOTP); + twoFactorAuthConfigManager.saveTwoFaAccountConfig(tenantId, user.getId(), totpTwoFaAccountConfig); + return totpTwoFaAccountConfig; + } + + private SmsTwoFactorAuthAccountConfig configureSmsTwoFa(Consumer... customizer) throws ThingsboardException { + SmsTwoFactorAuthProviderConfig smsTwoFaProviderConfig = new SmsTwoFactorAuthProviderConfig(); + smsTwoFaProviderConfig.setVerificationCodeLifetime(60); + smsTwoFaProviderConfig.setSmsVerificationMessageTemplate("${verificationCode}"); + Arrays.stream(customizer).forEach(c -> c.accept(smsTwoFaProviderConfig)); + + TwoFactorAuthSettings twoFaSettings = new TwoFactorAuthSettings(); + twoFaSettings.setUseSystemTwoFactorAuthSettings(false); + twoFaSettings.setProviders(Arrays.stream(new TwoFactorAuthProviderConfig[]{smsTwoFaProviderConfig}).collect(Collectors.toList())); + twoFactorAuthConfigManager.saveTwoFaSettings(tenantId, twoFaSettings); + + SmsTwoFactorAuthAccountConfig smsTwoFaAccountConfig = new SmsTwoFactorAuthAccountConfig(); + smsTwoFaAccountConfig.setPhoneNumber("+38050505050"); + twoFactorAuthConfigManager.saveTwoFaAccountConfig(tenantId, user.getId(), smsTwoFaAccountConfig); + return smsTwoFaAccountConfig; + } + + private String getCorrectTotp(TotpTwoFactorAuthAccountConfig totpTwoFaAccountConfig) { + String secret = StringUtils.substringAfterLast(totpTwoFaAccountConfig.getAuthUrl(), "secret="); + return new Totp(secret).now(); + } + }