bug: remove hardcoded JWT signing secret, load from JWT_SECRET env

README.md

@@ -36,7 +36,12 @@ And the code is organized as this:
 
 Integration with Spring Security and add other filter for jwt token process.
 
-The secret key is stored in `application.properties`.
+The JWT signing key is read from the `JWT_SECRET` environment variable (bound to `jwt.secret` in
+`application.properties`) and must be at least 64 bytes long. It is never committed to the repository. If
+`JWT_SECRET` is unset, a random key is generated at start up, which means every restart invalidates all
+previously issued tokens and multiple instances cannot share tokens, so always set it outside of local development:
+
+    export JWT_SECRET="$(openssl rand -base64 64 | tr -d '\n')"
 
 # Database
 

src/main/java/io/spring/infrastructure/service/DefaultJwtService.java

@@ -4,36 +4,63 @@
 import io.jsonwebtoken.Jws;
 import io.jsonwebtoken.Jwts;
 import io.jsonwebtoken.SignatureAlgorithm;
+import io.jsonwebtoken.security.Keys;
 import io.spring.core.service.JwtService;
 import io.spring.core.user.User;
+import java.nio.charset.StandardCharsets;
 import java.util.Date;
 import java.util.Optional;
 import javax.crypto.SecretKey;
 import javax.crypto.spec.SecretKeySpec;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.stereotype.Component;
 
 @Component
 public class DefaultJwtService implements JwtService {
+  private static final Logger log = LoggerFactory.getLogger(DefaultJwtService.class);
+  private static final int MINIMUM_SECRET_BYTES = 64;
+
   private final SecretKey signingKey;
   private final SignatureAlgorithm signatureAlgorithm;
   private int sessionTime;
 
   @Autowired
   public DefaultJwtService(
-      @Value("${jwt.secret}") String secret, @Value("${jwt.sessionTime}") int sessionTime) {
+      @Value("${jwt.secret:}") String secret, @Value("${jwt.sessionTime}") int sessionTime) {
     this.sessionTime = sessionTime;
-    signatureAlgorithm = SignatureAlgorithm.HS512;
-    this.signingKey = new SecretKeySpec(secret.getBytes(), signatureAlgorithm.getJcaName());
+    this.signatureAlgorithm = SignatureAlgorithm.HS512;
+    this.signingKey = buildSigningKey(secret, signatureAlgorithm);
+  }
+
+  private static SecretKey buildSigningKey(String secret, SignatureAlgorithm algorithm) {
+    if (secret == null || secret.trim().isEmpty()) {
+      log.warn(
+          "No JWT signing secret configured (set the JWT_SECRET environment variable to at least "
+              + "{} bytes). Generating a random key: tokens will be invalidated on every restart "
+              + "and will not be accepted by other instances.",
+          MINIMUM_SECRET_BYTES);
+      return Keys.secretKeyFor(algorithm);
+    }
+    byte[] keyBytes = secret.getBytes(StandardCharsets.UTF_8);
+    if (keyBytes.length < MINIMUM_SECRET_BYTES) {
+      throw new IllegalStateException(
+          "The JWT signing secret must be at least "
+              + MINIMUM_SECRET_BYTES
+              + " bytes long to be used with "
+              + algorithm.getValue());
+    }
+    return new SecretKeySpec(keyBytes, algorithm.getJcaName());
   }
 
   @Override
   public String toToken(User user) {
     return Jwts.builder()
         .setSubject(user.getId())
         .setExpiration(expireTimeFromNow())
-        .signWith(signingKey)
+        .signWith(signingKey, signatureAlgorithm)
         .compact();
   }
 

src/main/resources/application.properties

@@ -6,7 +6,7 @@ spring.jackson.deserialization.UNWRAP_ROOT_VALUE=true
 
 image.default=https://static.productionready.io/images/smiley-cyrus.jpg
 
-jwt.secret=nRvyYC4soFxBdZ-F-5Nnzz5USXstR1YylsTd-mA0aKtI9HUlriGrtkf-TiuDapkLiUCogO3JOK7kwZisrHp6wA
+jwt.secret=${JWT_SECRET:}
 jwt.sessionTime=86400
 
 mybatis.configuration.cache-enabled=true

src/test/java/io/spring/infrastructure/service/DefaultJwtServiceTest.java

@@ -9,11 +9,14 @@
 
 public class DefaultJwtServiceTest {
 
+  private static final String SECRET =
+      "1231231231231231231231231231231231231231231231231231231231231231";
+
   private JwtService jwtService;
 
   @BeforeEach
   public void setUp() {
-    jwtService = new DefaultJwtService("123123123123123123123123123123123123123123123123123123123123", 3600);
+    jwtService = new DefaultJwtService(SECRET, 3600);
   }
 
   @Test
@@ -38,4 +41,28 @@ public void should_get_null_with_expired_jwt() {
         "eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiJhaXNlbnNpeSIsImV4cCI6MTUwMjE2MTIwNH0.SJB-U60WzxLYNomqLo4G3v3LzFxJKuVrIud8D8Lz3-mgpo9pN1i7C8ikU_jQPJGm8HsC1CquGMI-rSuM7j6LDA";
     Assertions.assertFalse(jwtService.getSubFromToken(token).isPresent());
   }
+
+  @Test
+  public void should_reject_token_signed_with_another_secret() {
+    JwtService otherService =
+        new DefaultJwtService(
+            "3213213213213213213213213213213213213213213213213213213213213213", 3600);
+    String token = otherService.toToken(new User("email@email.com", "username", "123", "", ""));
+    Assertions.assertFalse(jwtService.getSubFromToken(token).isPresent());
+  }
+
+  @Test
+  public void should_reject_too_short_secret() {
+    Assertions.assertThrows(
+        IllegalStateException.class, () -> new DefaultJwtService("too-short-secret", 3600));
+  }
+
+  @Test
+  public void should_generate_random_key_when_no_secret_configured() {
+    User user = new User("email@email.com", "username", "123", "", "");
+    JwtService first = new DefaultJwtService("", 3600);
+    JwtService second = new DefaultJwtService("", 3600);
+    Assertions.assertTrue(first.getSubFromToken(first.toToken(user)).isPresent());
+    Assertions.assertFalse(second.getSubFromToken(first.toToken(user)).isPresent());
+  }
 }