From c15c4526aa6f81230ddc7ce48186abd312e540a7 Mon Sep 17 00:00:00 2001 From: dashevchenko Date: Wed, 15 Feb 2023 17:05:01 +0200 Subject: [PATCH 1/6] updated RNG used for reset password token to secure one --- .../server/controller/AuthController.java | 25 +++++++++++++++++++ .../src/main/resources/thingsboard.yml | 3 +++ .../server/dao/user/UserServiceImpl.java | 12 ++++++++- 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/application/src/main/java/org/thingsboard/server/controller/AuthController.java b/application/src/main/java/org/thingsboard/server/controller/AuthController.java index c73ac1ae66..9062ebda34 100644 --- a/application/src/main/java/org/thingsboard/server/controller/AuthController.java +++ b/application/src/main/java/org/thingsboard/server/controller/AuthController.java @@ -18,8 +18,10 @@ package org.thingsboard.server.controller; import com.fasterxml.jackson.databind.node.ObjectNode; import io.swagger.annotations.ApiOperation; import io.swagger.annotations.ApiParam; +import lombok.Getter; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.annotation.Value; import org.springframework.context.ApplicationEventPublisher; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; @@ -41,11 +43,13 @@ import org.thingsboard.server.common.data.edge.EdgeEventActionType; import org.thingsboard.server.common.data.exception.ThingsboardErrorCode; import org.thingsboard.server.common.data.exception.ThingsboardException; import org.thingsboard.server.common.data.id.TenantId; +import org.thingsboard.server.common.data.id.UserId; import org.thingsboard.server.common.data.security.UserCredentials; import org.thingsboard.server.common.data.security.event.UserCredentialsInvalidationEvent; import org.thingsboard.server.common.data.security.event.UserSessionInvalidationEvent; import org.thingsboard.server.common.data.security.model.SecuritySettings; import org.thingsboard.server.common.data.security.model.UserPasswordPolicy; +import org.thingsboard.server.common.msg.tools.TbRateLimits; import org.thingsboard.server.dao.audit.AuditLogService; import org.thingsboard.server.queue.util.TbCoreComponent; import org.thingsboard.server.service.security.auth.rest.RestAuthenticationDetails; @@ -62,6 +66,8 @@ import org.thingsboard.server.service.security.system.SystemSecurityService; import javax.servlet.http.HttpServletRequest; import java.net.URI; import java.net.URISyntaxException; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; @RestController @TbCoreComponent @@ -69,6 +75,10 @@ import java.net.URISyntaxException; @Slf4j @RequiredArgsConstructor public class AuthController extends BaseController { + @Value("${rate_limits.reset_password_per_user.configuration:5:300}") + @Getter + private String defaultLimitsConfiguration; + private final ConcurrentMap resetPasswordRateLimits = new ConcurrentHashMap<>(); private final BCryptPasswordEncoder passwordEncoder; private final JwtTokenFactory tokenFactory; private final MailService mailService; @@ -211,6 +221,12 @@ public class AuthController extends BaseController { HttpStatus responseStatus; String resetURI = "/login/resetPassword"; UserCredentials userCredentials = userService.findUserCredentialsByResetToken(TenantId.SYS_TENANT_ID, resetToken); + + TbRateLimits tbRateLimits = getTbRateLimits(userCredentials); + if (!tbRateLimits.tryConsume()) { + return ResponseEntity.status(HttpStatus.TOO_MANY_REQUESTS).build(); + } + if (userCredentials != null) { try { URI location = new URI(resetURI + "?resetToken=" + resetToken); @@ -323,4 +339,13 @@ public class AuthController extends BaseController { throw handleException(e); } } + + private TbRateLimits getTbRateLimits(UserCredentials userCredentials) { + TbRateLimits rateLimit = resetPasswordRateLimits.get(userCredentials.getUserId()); + if (rateLimit == null) { + rateLimit = new TbRateLimits(defaultLimitsConfiguration, true); + resetPasswordRateLimits.put(userCredentials.getUserId(), rateLimit); + } + return rateLimit; + } } diff --git a/application/src/main/resources/thingsboard.yml b/application/src/main/resources/thingsboard.yml index 5549861950..2f99b285fe 100644 --- a/application/src/main/resources/thingsboard.yml +++ b/application/src/main/resources/thingsboard.yml @@ -1209,3 +1209,6 @@ management: exposure: # Expose metrics endpoint (use value 'prometheus' to enable prometheus metrics). include: '${METRICS_ENDPOINTS_EXPOSE:info}' +rate_limits: + reset_password_per_user: + configuration: "${RESET_PASSWORD_PER_USER_RATE_LIMIT_CONFIGURATION:5:5}" diff --git a/dao/src/main/java/org/thingsboard/server/dao/user/UserServiceImpl.java b/dao/src/main/java/org/thingsboard/server/dao/user/UserServiceImpl.java index c8c5175e5f..e4dd882059 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/user/UserServiceImpl.java +++ b/dao/src/main/java/org/thingsboard/server/dao/user/UserServiceImpl.java @@ -47,6 +47,8 @@ import org.thingsboard.server.dao.exception.IncorrectParameterException; import org.thingsboard.server.dao.service.DataValidator; import org.thingsboard.server.dao.service.PaginatedRemover; +import java.security.SecureRandom; +import java.util.Base64; import java.util.HashMap; import java.util.Map; import java.util.Optional; @@ -192,10 +194,18 @@ public class UserServiceImpl extends AbstractEntityService implements UserServic if (!userCredentials.isEnabled()) { throw new DisabledException(String.format("User credentials not enabled [%s]", email)); } - userCredentials.setResetToken(StringUtils.randomAlphanumeric(DEFAULT_TOKEN_LENGTH)); + userCredentials.setResetToken(generateSafeToken()); return saveUserCredentials(tenantId, userCredentials); } + private String generateSafeToken() { + SecureRandom random = new SecureRandom(); + byte[] bytes = new byte[DEFAULT_TOKEN_LENGTH]; + random.nextBytes(bytes); + Base64.Encoder encoder = Base64.getUrlEncoder().withoutPadding(); + return encoder.encodeToString(bytes); + } + @Override public UserCredentials requestExpiredPasswordReset(TenantId tenantId, UserCredentialsId userCredentialsId) { UserCredentials userCredentials = userCredentialsDao.findById(tenantId, userCredentialsId.getId()); From 2c5c0a17bfdd3c2afe6000b443b56dd32ae8c87c Mon Sep 17 00:00:00 2001 From: dashevchenko Date: Wed, 15 Feb 2023 17:12:45 +0200 Subject: [PATCH 2/6] refactoring --- .../server/common/data/StringUtils.java | 11 +++++++++++ .../server/dao/user/UserServiceImpl.java | 19 ++++--------------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/common/data/src/main/java/org/thingsboard/server/common/data/StringUtils.java b/common/data/src/main/java/org/thingsboard/server/common/data/StringUtils.java index 6a2b0a58d6..7a4c180b9c 100644 --- a/common/data/src/main/java/org/thingsboard/server/common/data/StringUtils.java +++ b/common/data/src/main/java/org/thingsboard/server/common/data/StringUtils.java @@ -18,6 +18,9 @@ package org.thingsboard.server.common.data; import com.google.common.base.Splitter; import org.apache.commons.lang3.RandomStringUtils; +import java.security.SecureRandom; +import java.util.Base64; + import static org.apache.commons.lang3.StringUtils.repeat; public class StringUtils { @@ -180,4 +183,12 @@ public class StringUtils { return RandomStringUtils.randomAlphabetic(count); } + public static String generateSafeToken(int length) { + SecureRandom random = new SecureRandom(); + byte[] bytes = new byte[length]; + random.nextBytes(bytes); + Base64.Encoder encoder = Base64.getUrlEncoder().withoutPadding(); + return encoder.encodeToString(bytes); + } + } diff --git a/dao/src/main/java/org/thingsboard/server/dao/user/UserServiceImpl.java b/dao/src/main/java/org/thingsboard/server/dao/user/UserServiceImpl.java index e4dd882059..b4ad62c0fc 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/user/UserServiceImpl.java +++ b/dao/src/main/java/org/thingsboard/server/dao/user/UserServiceImpl.java @@ -29,7 +29,6 @@ import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; import org.thingsboard.common.util.JacksonUtil; import org.thingsboard.server.common.data.EntityType; -import org.thingsboard.server.common.data.StringUtils; import org.thingsboard.server.common.data.User; import org.thingsboard.server.common.data.id.CustomerId; import org.thingsboard.server.common.data.id.EntityId; @@ -40,19 +39,17 @@ import org.thingsboard.server.common.data.id.UserId; import org.thingsboard.server.common.data.page.PageData; import org.thingsboard.server.common.data.page.PageLink; import org.thingsboard.server.common.data.security.UserCredentials; -import org.thingsboard.server.common.data.security.UserSettings; import org.thingsboard.server.common.data.security.event.UserCredentialsInvalidationEvent; import org.thingsboard.server.dao.entity.AbstractEntityService; import org.thingsboard.server.dao.exception.IncorrectParameterException; import org.thingsboard.server.dao.service.DataValidator; import org.thingsboard.server.dao.service.PaginatedRemover; -import java.security.SecureRandom; -import java.util.Base64; import java.util.HashMap; import java.util.Map; import java.util.Optional; +import static org.thingsboard.server.common.data.StringUtils.generateSafeToken; import static org.thingsboard.server.dao.service.Validator.validateId; import static org.thingsboard.server.dao.service.Validator.validatePageLink; import static org.thingsboard.server.dao.service.Validator.validateString; @@ -128,7 +125,7 @@ public class UserServiceImpl extends AbstractEntityService implements UserServic if (user.getId() == null) { UserCredentials userCredentials = new UserCredentials(); userCredentials.setEnabled(false); - userCredentials.setActivateToken(StringUtils.randomAlphanumeric(DEFAULT_TOKEN_LENGTH)); + userCredentials.setActivateToken(generateSafeToken(DEFAULT_TOKEN_LENGTH)); userCredentials.setUserId(new UserId(savedUser.getUuidId())); saveUserCredentialsAndPasswordHistory(user.getTenantId(), userCredentials); } @@ -194,25 +191,17 @@ public class UserServiceImpl extends AbstractEntityService implements UserServic if (!userCredentials.isEnabled()) { throw new DisabledException(String.format("User credentials not enabled [%s]", email)); } - userCredentials.setResetToken(generateSafeToken()); + userCredentials.setResetToken(generateSafeToken(DEFAULT_TOKEN_LENGTH)); return saveUserCredentials(tenantId, userCredentials); } - private String generateSafeToken() { - SecureRandom random = new SecureRandom(); - byte[] bytes = new byte[DEFAULT_TOKEN_LENGTH]; - random.nextBytes(bytes); - Base64.Encoder encoder = Base64.getUrlEncoder().withoutPadding(); - return encoder.encodeToString(bytes); - } - @Override public UserCredentials requestExpiredPasswordReset(TenantId tenantId, UserCredentialsId userCredentialsId) { UserCredentials userCredentials = userCredentialsDao.findById(tenantId, userCredentialsId.getId()); if (!userCredentials.isEnabled()) { throw new IncorrectParameterException("Unable to reset password for inactive user"); } - userCredentials.setResetToken(StringUtils.randomAlphanumeric(DEFAULT_TOKEN_LENGTH)); + userCredentials.setResetToken(generateSafeToken(DEFAULT_TOKEN_LENGTH)); return saveUserCredentials(tenantId, userCredentials); } From 644f94d01288fc4c2a9411a874ae066a3e208742 Mon Sep 17 00:00:00 2001 From: dashevchenko Date: Wed, 15 Feb 2023 17:14:48 +0200 Subject: [PATCH 3/6] refactoring --- .../java/org/thingsboard/server/controller/AuthController.java | 2 +- application/src/main/resources/thingsboard.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/application/src/main/java/org/thingsboard/server/controller/AuthController.java b/application/src/main/java/org/thingsboard/server/controller/AuthController.java index 9062ebda34..336d3f8353 100644 --- a/application/src/main/java/org/thingsboard/server/controller/AuthController.java +++ b/application/src/main/java/org/thingsboard/server/controller/AuthController.java @@ -75,7 +75,7 @@ import java.util.concurrent.ConcurrentMap; @Slf4j @RequiredArgsConstructor public class AuthController extends BaseController { - @Value("${rate_limits.reset_password_per_user.configuration:5:300}") + @Value("${rate_limits.reset_password_per_user.configuration:5:3600}") @Getter private String defaultLimitsConfiguration; private final ConcurrentMap resetPasswordRateLimits = new ConcurrentHashMap<>(); diff --git a/application/src/main/resources/thingsboard.yml b/application/src/main/resources/thingsboard.yml index 2f99b285fe..7e08477419 100644 --- a/application/src/main/resources/thingsboard.yml +++ b/application/src/main/resources/thingsboard.yml @@ -1211,4 +1211,4 @@ management: include: '${METRICS_ENDPOINTS_EXPOSE:info}' rate_limits: reset_password_per_user: - configuration: "${RESET_PASSWORD_PER_USER_RATE_LIMIT_CONFIGURATION:5:5}" + configuration: "${RESET_PASSWORD_PER_USER_RATE_LIMIT_CONFIGURATION:5:3600}" From ab643063f253eb018a7f600a47035532a7b041ec Mon Sep 17 00:00:00 2001 From: dashevchenko Date: Thu, 16 Feb 2023 11:19:42 +0200 Subject: [PATCH 4/6] refactoring --- .../server/controller/AuthController.java | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/application/src/main/java/org/thingsboard/server/controller/AuthController.java b/application/src/main/java/org/thingsboard/server/controller/AuthController.java index 336d3f8353..88de329883 100644 --- a/application/src/main/java/org/thingsboard/server/controller/AuthController.java +++ b/application/src/main/java/org/thingsboard/server/controller/AuthController.java @@ -18,7 +18,6 @@ package org.thingsboard.server.controller; import com.fasterxml.jackson.databind.node.ObjectNode; import io.swagger.annotations.ApiOperation; import io.swagger.annotations.ApiParam; -import lombok.Getter; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Value; @@ -75,8 +74,8 @@ import java.util.concurrent.ConcurrentMap; @Slf4j @RequiredArgsConstructor public class AuthController extends BaseController { + @Value("${rate_limits.reset_password_per_user.configuration:5:3600}") - @Getter private String defaultLimitsConfiguration; private final ConcurrentMap resetPasswordRateLimits = new ConcurrentHashMap<>(); private final BCryptPasswordEncoder passwordEncoder; @@ -222,12 +221,11 @@ public class AuthController extends BaseController { String resetURI = "/login/resetPassword"; UserCredentials userCredentials = userService.findUserCredentialsByResetToken(TenantId.SYS_TENANT_ID, resetToken); - TbRateLimits tbRateLimits = getTbRateLimits(userCredentials); - if (!tbRateLimits.tryConsume()) { - return ResponseEntity.status(HttpStatus.TOO_MANY_REQUESTS).build(); - } - if (userCredentials != null) { + TbRateLimits tbRateLimits = getTbRateLimits(userCredentials.getUserId()); + if (!tbRateLimits.tryConsume()) { + return ResponseEntity.status(HttpStatus.TOO_MANY_REQUESTS).build(); + } try { URI location = new URI(resetURI + "?resetToken=" + resetToken); headers.setLocation(location); @@ -340,12 +338,8 @@ public class AuthController extends BaseController { } } - private TbRateLimits getTbRateLimits(UserCredentials userCredentials) { - TbRateLimits rateLimit = resetPasswordRateLimits.get(userCredentials.getUserId()); - if (rateLimit == null) { - rateLimit = new TbRateLimits(defaultLimitsConfiguration, true); - resetPasswordRateLimits.put(userCredentials.getUserId(), rateLimit); - } - return rateLimit; + private TbRateLimits getTbRateLimits(UserId userId) { + return resetPasswordRateLimits.computeIfAbsent(userId, + key -> new TbRateLimits(defaultLimitsConfiguration, true)); } } From 831018af40795f7908b66a1ea2ff8b8c09d01160 Mon Sep 17 00:00:00 2001 From: dashevchenko Date: Fri, 17 Feb 2023 14:41:20 +0200 Subject: [PATCH 5/6] minor refactoring --- .../java/org/thingsboard/server/controller/AuthController.java | 2 +- application/src/main/resources/thingsboard.yml | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/application/src/main/java/org/thingsboard/server/controller/AuthController.java b/application/src/main/java/org/thingsboard/server/controller/AuthController.java index 88de329883..d0e382f888 100644 --- a/application/src/main/java/org/thingsboard/server/controller/AuthController.java +++ b/application/src/main/java/org/thingsboard/server/controller/AuthController.java @@ -75,7 +75,7 @@ import java.util.concurrent.ConcurrentMap; @RequiredArgsConstructor public class AuthController extends BaseController { - @Value("${rate_limits.reset_password_per_user.configuration:5:3600}") + @Value("${rate_limits.reset_password_per_user:5:3600}") private String defaultLimitsConfiguration; private final ConcurrentMap resetPasswordRateLimits = new ConcurrentHashMap<>(); private final BCryptPasswordEncoder passwordEncoder; diff --git a/application/src/main/resources/thingsboard.yml b/application/src/main/resources/thingsboard.yml index 7e08477419..7a5a1e57fd 100644 --- a/application/src/main/resources/thingsboard.yml +++ b/application/src/main/resources/thingsboard.yml @@ -1210,5 +1210,4 @@ management: # Expose metrics endpoint (use value 'prometheus' to enable prometheus metrics). include: '${METRICS_ENDPOINTS_EXPOSE:info}' rate_limits: - reset_password_per_user: - configuration: "${RESET_PASSWORD_PER_USER_RATE_LIMIT_CONFIGURATION:5:3600}" + reset_password_per_user: "${RESET_PASSWORD_PER_USER_RATE_LIMIT_CONFIGURATION:5:3600}" From 04c36916edb1de7666851474a79d764b7ec76b04 Mon Sep 17 00:00:00 2001 From: dashevchenko Date: Tue, 28 Feb 2023 14:53:57 +0200 Subject: [PATCH 6/6] moved system env property --- .../org/thingsboard/server/controller/AuthController.java | 2 +- application/src/main/resources/thingsboard.yml | 5 +++-- .../java/org/thingsboard/server/common/data/StringUtils.java | 5 +++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/application/src/main/java/org/thingsboard/server/controller/AuthController.java b/application/src/main/java/org/thingsboard/server/controller/AuthController.java index d0e382f888..113c292380 100644 --- a/application/src/main/java/org/thingsboard/server/controller/AuthController.java +++ b/application/src/main/java/org/thingsboard/server/controller/AuthController.java @@ -75,7 +75,7 @@ import java.util.concurrent.ConcurrentMap; @RequiredArgsConstructor public class AuthController extends BaseController { - @Value("${rate_limits.reset_password_per_user:5:3600}") + @Value("${server.rest.rate_limits.reset_password_per_user:5:3600}") private String defaultLimitsConfiguration; private final ConcurrentMap resetPasswordRateLimits = new ConcurrentHashMap<>(); private final BCryptPasswordEncoder passwordEncoder; diff --git a/application/src/main/resources/thingsboard.yml b/application/src/main/resources/thingsboard.yml index 7a5a1e57fd..d5d3a9dbd0 100644 --- a/application/src/main/resources/thingsboard.yml +++ b/application/src/main/resources/thingsboard.yml @@ -73,6 +73,8 @@ server: min_timeout: "${MIN_SERVER_SIDE_RPC_TIMEOUT:5000}" # Default value of the server side RPC timeout. default_timeout: "${DEFAULT_SERVER_SIDE_RPC_TIMEOUT:10000}" + rate_limits: + reset_password_per_user: "${RESET_PASSWORD_PER_USER_RATE_LIMIT_CONFIGURATION:5:3600}" # Application info app: @@ -1209,5 +1211,4 @@ management: exposure: # Expose metrics endpoint (use value 'prometheus' to enable prometheus metrics). include: '${METRICS_ENDPOINTS_EXPOSE:info}' -rate_limits: - reset_password_per_user: "${RESET_PASSWORD_PER_USER_RATE_LIMIT_CONFIGURATION:5:3600}" + diff --git a/common/data/src/main/java/org/thingsboard/server/common/data/StringUtils.java b/common/data/src/main/java/org/thingsboard/server/common/data/StringUtils.java index 7a4c180b9c..3b38aa57c1 100644 --- a/common/data/src/main/java/org/thingsboard/server/common/data/StringUtils.java +++ b/common/data/src/main/java/org/thingsboard/server/common/data/StringUtils.java @@ -24,6 +24,8 @@ import java.util.Base64; import static org.apache.commons.lang3.StringUtils.repeat; public class StringUtils { + public static final SecureRandom RANDOM = new SecureRandom(); + public static final String EMPTY = ""; public static final int INDEX_NOT_FOUND = -1; @@ -184,9 +186,8 @@ public class StringUtils { } public static String generateSafeToken(int length) { - SecureRandom random = new SecureRandom(); byte[] bytes = new byte[length]; - random.nextBytes(bytes); + RANDOM.nextBytes(bytes); Base64.Encoder encoder = Base64.getUrlEncoder().withoutPadding(); return encoder.encodeToString(bytes); }