From b95f1c95e09c23a0c552c660f25521440233136c Mon Sep 17 00:00:00 2001
From: Sergey Matvienko <smatvienko@thingsboard.io>
Date: Thu, 10 Nov 2022 18:00:54 +0100
Subject: [PATCH] jwt settings workout on review feedback

---
 .../config/jwt/JwtSettingsServiceDefault.java |  6 +++++-
 .../config/jwt/JwtSettingsValidator.java      | 19 ++++++++++++++-----
 .../processing/AbstractConsumerService.java   |  1 +
 3 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/application/src/main/java/org/thingsboard/server/config/jwt/JwtSettingsServiceDefault.java b/application/src/main/java/org/thingsboard/server/config/jwt/JwtSettingsServiceDefault.java
index d65edaa93b..f64b7c4658 100644
--- a/application/src/main/java/org/thingsboard/server/config/jwt/JwtSettingsServiceDefault.java
+++ b/application/src/main/java/org/thingsboard/server/config/jwt/JwtSettingsServiceDefault.java
@@ -164,7 +164,11 @@ public class JwtSettingsServiceDefault implements JwtSettingsService {
             if (isAllowedDefaultJwtSigningKey()) {
                 log.warn("Default JWT signing key is allowed. This is a security issue. Please, consider to set a strong key in admin settings");
             } else {
-                String message = "Please, set a unique signing key with env variable JWT_TOKEN_SIGNING_KEY. Key is a Base64 encoded phrase. This will require to generate new tokens for all users and API that uses JWT tokens. To allow insecure JWS use TB_ALLOW_DEFAULT_JWT_SIGNING_KEY=true";
+                String message = "UPGRADE ERROR. YOUR ACTION REQUIRED. Please, set a unique signing key with env variable JWT_TOKEN_SIGNING_KEY. " +
+                        "The key should be a Base64 encoded string representing at least 256 bits of data. " +
+                        "This will require to generate new tokens for all UI users and scripts that use JWT. " +
+                        "To keep the default non-secure JWT signing key set TB_ALLOW_DEFAULT_JWT_SIGNING_KEY=true and restart the upgrade. " +
+                        "You may change the JWT signing key later in the Admin Settings UI.";
                 log.error(message);
                 throw new ValidationException(message);
             }
diff --git a/application/src/main/java/org/thingsboard/server/config/jwt/JwtSettingsValidator.java b/application/src/main/java/org/thingsboard/server/config/jwt/JwtSettingsValidator.java
index 4e91c654e0..c6ec252d9f 100644
--- a/application/src/main/java/org/thingsboard/server/config/jwt/JwtSettingsValidator.java
+++ b/application/src/main/java/org/thingsboard/server/config/jwt/JwtSettingsValidator.java
@@ -16,6 +16,7 @@
 package org.thingsboard.server.config.jwt;
 
 import lombok.AllArgsConstructor;
+import org.apache.commons.lang3.RandomUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.bouncycastle.util.Arrays;
 import org.springframework.stereotype.Component;
@@ -23,6 +24,7 @@ import org.thingsboard.server.dao.exception.DataValidationException;
 
 import java.util.Base64;
 import java.util.Optional;
+import java.util.concurrent.TimeUnit;
 
 @Component
 @AllArgsConstructor
@@ -32,11 +34,14 @@ public class JwtSettingsValidator {
         if (StringUtils.isEmpty(jwtSettings.getTokenIssuer())) {
             throw new DataValidationException("JWT token issuer should be specified!");
         }
-        if (Optional.ofNullable(jwtSettings.getRefreshTokenExpTime()).orElse(0) <= 0) {
-            throw new DataValidationException("JWT refresh token expiration time should be specified!");
+        if (Optional.ofNullable(jwtSettings.getRefreshTokenExpTime()).orElse(0) <= TimeUnit.MINUTES.toSeconds(15)) {
+            throw new DataValidationException("JWT refresh token expiration time should be at least 15 minutes!");
         }
-        if (Optional.ofNullable(jwtSettings.getTokenExpirationTime()).orElse(0) <= 0) {
-            throw new DataValidationException("JWT token expiration time should be specified!");
+        if (Optional.ofNullable(jwtSettings.getTokenExpirationTime()).orElse(0) <= TimeUnit.MINUTES.toSeconds(1)) {
+            throw new DataValidationException("JWT token expiration time should be at least 1 minute!");
+        }
+        if (jwtSettings.getTokenExpirationTime() >= jwtSettings.getRefreshTokenExpTime()) {
+            throw new DataValidationException("JWT token expiration time should greater than JWT refresh token expiration time!");
         }
         if (StringUtils.isEmpty(jwtSettings.getTokenSigningKey())) {
             throw new DataValidationException("JWT token signing key should be specified!");
@@ -52,7 +57,11 @@ public class JwtSettingsValidator {
         if (Arrays.isNullOrEmpty(decodedKey)) {
             throw new DataValidationException("JWT token signing key should be non-empty after Base64 decoding!");
         }
-        Arrays.fill(decodedKey, (byte) 0);
+        if (decodedKey.length * Byte.SIZE < 256) {
+            throw new DataValidationException("JWT token signing key should be a Base64 encoded string representing at least 256 bits of data!");
+        }
+
+        System.arraycopy(decodedKey, 0, RandomUtils.nextBytes(decodedKey.length), 0, decodedKey.length); //secure memory
     }
 
 }
diff --git a/application/src/main/java/org/thingsboard/server/service/queue/processing/AbstractConsumerService.java b/application/src/main/java/org/thingsboard/server/service/queue/processing/AbstractConsumerService.java
index df7f0922ea..8d3ec4b55a 100644
--- a/application/src/main/java/org/thingsboard/server/service/queue/processing/AbstractConsumerService.java
+++ b/application/src/main/java/org/thingsboard/server/service/queue/processing/AbstractConsumerService.java
@@ -178,6 +178,7 @@ public abstract class AbstractConsumerService<N extends com.google.protobuf.Gene
                 } else if (EntityType.TENANT.equals(componentLifecycleMsg.getEntityId().getEntityType())) {
                     if (TenantId.SYS_TENANT_ID.equals(componentLifecycleMsg.getTenantId())) {
                         jwtSettingsService.ifPresent(JwtSettingsService::reloadJwtSettings);
+                        return;
                     } else {
                         tenantProfileCache.evict(componentLifecycleMsg.getTenantId());
                         partitionService.removeTenant(componentLifecycleMsg.getTenantId());