diff --git a/application/src/main/java/org/thingsboard/server/exception/ThingsboardErrorResponseHandler.java b/application/src/main/java/org/thingsboard/server/exception/ThingsboardErrorResponseHandler.java index 0cd581b2e5..dfdcf16090 100644 --- a/application/src/main/java/org/thingsboard/server/exception/ThingsboardErrorResponseHandler.java +++ b/application/src/main/java/org/thingsboard/server/exception/ThingsboardErrorResponseHandler.java @@ -178,7 +178,7 @@ public class ThingsboardErrorResponseHandler extends ResponseEntityExceptionHand private void handleAuthenticationException(AuthenticationException authenticationException, HttpServletResponse response) throws IOException { response.setStatus(HttpStatus.UNAUTHORIZED.value()); if (authenticationException instanceof BadCredentialsException || authenticationException instanceof UsernameNotFoundException) { - JacksonUtil.writeValue(response.getWriter(), ThingsboardErrorResponse.of("Invalid username or password", ThingsboardErrorCode.AUTHENTICATION, HttpStatus.UNAUTHORIZED)); + JacksonUtil.writeValue(response.getWriter(), ThingsboardErrorResponse.of(authenticationException.getMessage().isEmpty() ? "Invalid username or password" : authenticationException.getMessage(), ThingsboardErrorCode.AUTHENTICATION, HttpStatus.UNAUTHORIZED)); } else if (authenticationException instanceof DisabledException) { JacksonUtil.writeValue(response.getWriter(), ThingsboardErrorResponse.of("User account is not active", ThingsboardErrorCode.AUTHENTICATION, HttpStatus.UNAUTHORIZED)); } else if (authenticationException instanceof LockedException) { diff --git a/application/src/main/java/org/thingsboard/server/service/security/system/DefaultSystemSecurityService.java b/application/src/main/java/org/thingsboard/server/service/security/system/DefaultSystemSecurityService.java index 2c2956b288..9fe24eaf01 100644 --- a/application/src/main/java/org/thingsboard/server/service/security/system/DefaultSystemSecurityService.java +++ b/application/src/main/java/org/thingsboard/server/service/security/system/DefaultSystemSecurityService.java @@ -107,6 +107,7 @@ public class DefaultSystemSecurityService implements SystemSecurityService { securitySettings = new SecuritySettings(); securitySettings.setPasswordPolicy(new UserPasswordPolicy()); securitySettings.getPasswordPolicy().setMinimumLength(6); + securitySettings.getPasswordPolicy().setMaximumLength(72); } return securitySettings; } @@ -131,9 +132,18 @@ public class DefaultSystemSecurityService implements SystemSecurityService { @Override public void validateUserCredentials(TenantId tenantId, UserCredentials userCredentials, String username, String password) throws AuthenticationException { + SecuritySettings securitySettings = self.getSecuritySettings(tenantId); + UserPasswordPolicy passwordPolicy = securitySettings.getPasswordPolicy(); + + if (passwordPolicy.getForceUserToResetPasswordIfNotValid()) { + try { + validatePasswordByPolicy(password, passwordPolicy); + } catch (DataValidationException e) { + throw new BadCredentialsException("Password does not pass validation. Please try again or reset password to valid one."); + } + } if (!encoder.matches(password, userCredentials.getPassword())) { int failedLoginAttempts = userService.increaseFailedLoginAttempts(tenantId, userCredentials.getUserId()); - SecuritySettings securitySettings = self.getSecuritySettings(tenantId); if (securitySettings.getMaxFailedLoginAttempts() != null && securitySettings.getMaxFailedLoginAttempts() > 0) { if (failedLoginAttempts > securitySettings.getMaxFailedLoginAttempts() && userCredentials.isEnabled()) { lockAccount(userCredentials.getUserId(), username, securitySettings.getUserLockoutNotificationEmail(), securitySettings.getMaxFailedLoginAttempts()); @@ -149,7 +159,6 @@ public class DefaultSystemSecurityService implements SystemSecurityService { userService.resetFailedLoginAttempts(tenantId, userCredentials.getUserId()); - SecuritySettings securitySettings = self.getSecuritySettings(tenantId); if (isPositiveInteger(securitySettings.getPasswordPolicy().getPasswordExpirationPeriodDays())) { if ((userCredentials.getCreatedTime() + TimeUnit.DAYS.toMillis(securitySettings.getPasswordPolicy().getPasswordExpirationPeriodDays())) @@ -199,8 +208,26 @@ public class DefaultSystemSecurityService implements SystemSecurityService { SecuritySettings securitySettings = self.getSecuritySettings(tenantId); UserPasswordPolicy passwordPolicy = securitySettings.getPasswordPolicy(); + validatePasswordByPolicy(password, passwordPolicy); + + if (userCredentials != null && isPositiveInteger(passwordPolicy.getPasswordReuseFrequencyDays())) { + long passwordReuseFrequencyTs = System.currentTimeMillis() - TimeUnit.DAYS.toMillis(passwordPolicy.getPasswordReuseFrequencyDays()); + JsonNode additionalInfo = userCredentials.getAdditionalInfo(); + if (additionalInfo instanceof ObjectNode && additionalInfo.has(UserServiceImpl.USER_PASSWORD_HISTORY)) { + JsonNode userPasswordHistoryJson = additionalInfo.get(UserServiceImpl.USER_PASSWORD_HISTORY); + Map userPasswordHistoryMap = JacksonUtil.convertValue(userPasswordHistoryJson, new TypeReference<>() {}); + for (Map.Entry entry : userPasswordHistoryMap.entrySet()) { + if (encoder.matches(password, entry.getValue()) && Long.parseLong(entry.getKey()) > passwordReuseFrequencyTs) { + throw new DataValidationException("Password was already used for the last " + passwordPolicy.getPasswordReuseFrequencyDays() + " days"); + } + } + } + } + } + + private void validatePasswordByPolicy(String password, UserPasswordPolicy passwordPolicy) { List passwordRules = new ArrayList<>(); - passwordRules.add(new LengthRule(passwordPolicy.getMinimumLength(), Integer.MAX_VALUE)); + passwordRules.add(new LengthRule(passwordPolicy.getMinimumLength(), passwordPolicy.getMaximumLength())); if (isPositiveInteger(passwordPolicy.getMinimumUppercaseLetters())) { passwordRules.add(new CharacterRule(EnglishCharacterData.UpperCase, passwordPolicy.getMinimumUppercaseLetters())); } @@ -223,21 +250,6 @@ public class DefaultSystemSecurityService implements SystemSecurityService { String message = String.join("\n", validator.getMessages(result)); throw new DataValidationException(message); } - - if (userCredentials != null && isPositiveInteger(passwordPolicy.getPasswordReuseFrequencyDays())) { - long passwordReuseFrequencyTs = System.currentTimeMillis() - TimeUnit.DAYS.toMillis(passwordPolicy.getPasswordReuseFrequencyDays()); - JsonNode additionalInfo = userCredentials.getAdditionalInfo(); - if (additionalInfo instanceof ObjectNode && additionalInfo.has(UserServiceImpl.USER_PASSWORD_HISTORY)) { - JsonNode userPasswordHistoryJson = additionalInfo.get(UserServiceImpl.USER_PASSWORD_HISTORY); - Map userPasswordHistoryMap = JacksonUtil.convertValue(userPasswordHistoryJson, new TypeReference<>() {}); - for (Map.Entry entry : userPasswordHistoryMap.entrySet()) { - if (encoder.matches(password, entry.getValue()) && Long.parseLong(entry.getKey()) > passwordReuseFrequencyTs) { - throw new DataValidationException("Password was already used for the last " + passwordPolicy.getPasswordReuseFrequencyDays() + " days"); - } - } - - } - } } @Override diff --git a/application/src/test/java/org/thingsboard/server/controller/AuthControllerTest.java b/application/src/test/java/org/thingsboard/server/controller/AuthControllerTest.java index b833e45d34..099f6d8817 100644 --- a/application/src/test/java/org/thingsboard/server/controller/AuthControllerTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/AuthControllerTest.java @@ -15,19 +15,41 @@ */ package org.thingsboard.server.controller; +import com.fasterxml.jackson.databind.JsonNode; +import org.junit.After; import org.junit.Test; +import org.mockito.Mockito; +import org.springframework.http.HttpHeaders; +import org.testcontainers.shaded.org.apache.commons.lang3.RandomStringUtils; +import org.thingsboard.common.util.JacksonUtil; import org.thingsboard.server.common.data.security.Authority; +import org.thingsboard.server.common.data.security.model.SecuritySettings; import org.thingsboard.server.dao.service.DaoSqlTest; +import org.thingsboard.server.service.security.auth.rest.LoginRequest; +import org.thingsboard.server.service.security.model.ChangePasswordRequest; import java.util.concurrent.TimeUnit; import static org.hamcrest.Matchers.is; +import static org.mockito.ArgumentMatchers.anyString; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @DaoSqlTest public class AuthControllerTest extends AbstractControllerTest { + @After + public void tearDown() throws Exception { + loginSysAdmin(); + SecuritySettings securitySettings = doGet("/api/admin/securitySettings", SecuritySettings.class); + + securitySettings.getPasswordPolicy().setMaximumLength(72); + securitySettings.getPasswordPolicy().setForceUserToResetPasswordIfNotValid(false); + + doPost("/api/admin/securitySettings", securitySettings).andExpect(status().isOk()); + } + @Test public void testGetUser() throws Exception { @@ -84,4 +106,65 @@ public class AuthControllerTest extends AbstractControllerTest { .andExpect(jsonPath("$.authority",is(Authority.SYS_ADMIN.name()))) .andExpect(jsonPath("$.email",is(SYS_ADMIN_EMAIL))); } + + @Test + public void testShouldNotUpdatePasswordWithValueLongerThanDefaultLimit() throws Exception { + loginTenantAdmin(); + ChangePasswordRequest changePasswordRequest = new ChangePasswordRequest(); + changePasswordRequest.setCurrentPassword("tenant"); + changePasswordRequest.setNewPassword(RandomStringUtils.randomAlphanumeric(73)); + doPost("/api/auth/changePassword", changePasswordRequest) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.message", is("Password must be no more than 72 characters in length."))); + } + + @Test + public void testShouldNotAuthorizeUserIfHisPasswordBecameTooLong() throws Exception { + loginTenantAdmin(); + + ChangePasswordRequest changePasswordRequest = new ChangePasswordRequest(); + changePasswordRequest.setCurrentPassword("tenant"); + String newPassword = RandomStringUtils.randomAlphanumeric(16); + changePasswordRequest.setNewPassword(newPassword); + doPost("/api/auth/changePassword", changePasswordRequest) + .andExpect(status().isOk()); + loginUser(TENANT_ADMIN_EMAIL, newPassword); + + loginSysAdmin(); + SecuritySettings securitySettings = doGet("/api/admin/securitySettings", SecuritySettings.class); + securitySettings.getPasswordPolicy().setMaximumLength(15); + securitySettings.getPasswordPolicy().setForceUserToResetPasswordIfNotValid(true); + doPost("/api/admin/securitySettings", securitySettings).andExpect(status().isOk()); + + //try to login with user password that is not valid after security settings was updated + doPost("/api/auth/login", new LoginRequest(TENANT_ADMIN_EMAIL, newPassword)) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.message", is("Password does not pass validation. Please try again or reset password to valid one."))); + } + + @Test + public void testShouldNotResetPasswordToTooLongValue() throws Exception { + loginTenantAdmin(); + + JsonNode resetPasswordByEmailRequest = JacksonUtil.newObjectNode() + .put("email", TENANT_ADMIN_EMAIL); + + doPost("/api/noauth/resetPasswordByEmail", resetPasswordByEmailRequest) + .andExpect(status().isOk()); + Thread.sleep(1000); + doGet("/api/noauth/resetPassword?resetToken={resetToken}", this.currentResetPasswordToken) + .andExpect(status().isSeeOther()) + .andExpect(header().string(HttpHeaders.LOCATION, "/login/resetPassword?resetToken=" + this.currentResetPasswordToken)); + + String newPassword = RandomStringUtils.randomAlphanumeric(73); + JsonNode resetPasswordRequest = JacksonUtil.newObjectNode() + .put("resetToken", this.currentResetPasswordToken) + .put("password", newPassword); + + Mockito.doNothing().when(mailService).sendPasswordWasResetEmail(anyString(), anyString()); + doPost("/api/noauth/resetPassword", resetPasswordRequest) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.message", + is("Password must be no more than 72 characters in length."))); + } } diff --git a/common/data/src/main/java/org/thingsboard/server/common/data/security/model/UserPasswordPolicy.java b/common/data/src/main/java/org/thingsboard/server/common/data/security/model/UserPasswordPolicy.java index 881af89e27..fd067287b5 100644 --- a/common/data/src/main/java/org/thingsboard/server/common/data/security/model/UserPasswordPolicy.java +++ b/common/data/src/main/java/org/thingsboard/server/common/data/security/model/UserPasswordPolicy.java @@ -27,6 +27,8 @@ public class UserPasswordPolicy implements Serializable { @ApiModelProperty(position = 1, value = "Minimum number of symbols in the password." ) private Integer minimumLength; + @ApiModelProperty(position = 1, value = "Maximum number of symbols in the password." ) + private Integer maximumLength; @ApiModelProperty(position = 1, value = "Minimum number of uppercase letters in the password." ) private Integer minimumUppercaseLetters; @ApiModelProperty(position = 1, value = "Minimum number of lowercase letters in the password." ) @@ -37,6 +39,8 @@ public class UserPasswordPolicy implements Serializable { private Integer minimumSpecialCharacters; @ApiModelProperty(position = 1, value = "Allow whitespaces") private Boolean allowWhitespaces = true; + @ApiModelProperty(position = 1, value = "Force user to update password if existing one does not pass validation") + private Boolean forceUserToResetPasswordIfNotValid = false; @ApiModelProperty(position = 1, value = "Password expiration period (days). Force expiration of the password." ) private Integer passwordExpirationPeriodDays;