diff --git a/app/common/src/main/java/stirling/software/common/configuration/InstallationPathConfig.java b/app/common/src/main/java/stirling/software/common/configuration/InstallationPathConfig.java index 08618329d..ebea350fc 100644 --- a/app/common/src/main/java/stirling/software/common/configuration/InstallationPathConfig.java +++ b/app/common/src/main/java/stirling/software/common/configuration/InstallationPathConfig.java @@ -24,6 +24,7 @@ public class InstallationPathConfig { private static final String STATIC_PATH; private static final String TEMPLATES_PATH; private static final String SIGNATURES_PATH; + private static final String PRIVATE_KEY_PATH; static { BASE_PATH = initializeBasePath(); @@ -43,6 +44,7 @@ public class InstallationPathConfig { STATIC_PATH = CUSTOM_FILES_PATH + "static" + File.separator; TEMPLATES_PATH = CUSTOM_FILES_PATH + "templates" + File.separator; SIGNATURES_PATH = CUSTOM_FILES_PATH + "signatures" + File.separator; + PRIVATE_KEY_PATH = CUSTOM_FILES_PATH + "keys" + File.separator; } private static String initializeBasePath() { @@ -114,4 +116,8 @@ public class InstallationPathConfig { public static String getSignaturesPath() { return SIGNATURES_PATH; } + + public static String getPrivateKeyPath() { + return PRIVATE_KEY_PATH; + } } diff --git a/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java b/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java index aeb220dae..48df4f948 100644 --- a/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java +++ b/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java @@ -303,6 +303,9 @@ public class ApplicationProperties { public static class Jwt { private boolean enableKeystore = true; private boolean enableKeyRotation = false; + private boolean enableKeyCleanup = true; + private int keyRetentionDays = 7; + private int cleanupBatchSize = 100; } } diff --git a/app/core/src/main/resources/templates/account.html b/app/core/src/main/resources/templates/account.html index 33a0d9f47..db48bb3a5 100644 --- a/app/core/src/main/resources/templates/account.html +++ b/app/core/src/main/resources/templates/account.html @@ -390,8 +390,13 @@ key.includes('clientSubmissionOrder') || key.includes('lastSubmitTime') || key.includes('lastClientId') || - - + key.includes('stirling_jwt') || + key.includes('JSESSIONID') || + key.includes('XSRF-TOKEN') || + key.includes('remember-me') || + key.includes('auth') || + key.includes('token') || + key.includes('session') || key.includes('posthog') || key.includes('ssoRedirectAttempts') || key.includes('lastRedirectAttempt') || key.includes('surveyVersion') || key.includes('pageViews'); } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/configuration/SecurityConfiguration.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/configuration/SecurityConfiguration.java index a24b109c9..ed309afda 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/configuration/SecurityConfiguration.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/configuration/SecurityConfiguration.java @@ -250,6 +250,8 @@ public class SecurityConfiguration { || trimmedUri.startsWith("/css/") || trimmedUri.startsWith("/fonts/") || trimmedUri.startsWith("/js/") + || trimmedUri.startsWith("/pdfjs/") + || trimmedUri.startsWith("/pdfjs-legacy/") || trimmedUri.startsWith("/favicon") || trimmedUri.startsWith( "/api/v1/info/status") diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/database/repository/JwtSigningKeyRepository.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/database/repository/JwtSigningKeyRepository.java index cde43ff69..ee7c3b419 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/database/repository/JwtSigningKeyRepository.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/database/repository/JwtSigningKeyRepository.java @@ -1,8 +1,14 @@ package stirling.software.proprietary.security.database.repository; +import java.time.LocalDateTime; +import java.util.List; import java.util.Optional; +import org.springframework.data.domain.Pageable; import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Modifying; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; import org.springframework.stereotype.Repository; import stirling.software.proprietary.security.model.JwtSigningKey; @@ -14,5 +20,16 @@ public interface JwtSigningKeyRepository extends JpaRepository findByKeyId(String keyId); - Optional findByKeyIdAndIsActiveTrue(String keyId); + @Query( + "SELECT k FROM signing_keys k WHERE k.isActive = false AND k.createdAt < :cutoffDate ORDER BY k.createdAt ASC") + List findInactiveKeysOlderThan( + @Param("cutoffDate") LocalDateTime cutoffDate, Pageable pageable); + + @Query( + "SELECT COUNT(k) FROM signing_keys k WHERE k.isActive = false AND k.createdAt < :cutoffDate") + long countKeysEligibleForCleanup(@Param("cutoffDate") LocalDateTime cutoffDate); + + @Modifying + @Query("DELETE FROM signing_keys k WHERE k.id IN :ids") + void deleteAllByIdInBatch(@Param("ids") List ids); } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/filter/UserAuthenticationFilter.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/filter/UserAuthenticationFilter.java index 70c9e233e..f6074ceff 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/filter/UserAuthenticationFilter.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/filter/UserAuthenticationFilter.java @@ -124,11 +124,10 @@ public class UserAuthenticationFilter extends OncePerRequestFilter { response.getWriter() .write( """ - Authentication required. Please provide a X-API-KEY in request\ - header. + Authentication required. Please provide a X-API-KEY in request header. This is found in Settings -> Account Settings -> API Key - Alternatively you can disable authentication if this is\ - unexpected"""); + Alternatively you can disable authentication if this is unexpected. + """); } return; } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtKeyCleanupService.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtKeyCleanupService.java new file mode 100644 index 000000000..706f7a537 --- /dev/null +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtKeyCleanupService.java @@ -0,0 +1,133 @@ +package stirling.software.proprietary.security.service; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.time.LocalDateTime; +import java.util.List; +import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.domain.PageRequest; +import org.springframework.data.domain.Pageable; +import org.springframework.scheduling.annotation.Scheduled; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +import lombok.extern.slf4j.Slf4j; + +import stirling.software.common.configuration.InstallationPathConfig; +import stirling.software.common.model.ApplicationProperties; +import stirling.software.proprietary.security.database.repository.JwtSigningKeyRepository; +import stirling.software.proprietary.security.model.JwtSigningKey; + +@Slf4j +@Service +public class JwtKeyCleanupService { + + private final JwtSigningKeyRepository signingKeyRepository; + private final JwtKeystoreService keystoreService; + private final ApplicationProperties.Security.Jwt jwtProperties; + + @Autowired + public JwtKeyCleanupService( + JwtSigningKeyRepository signingKeyRepository, + JwtKeystoreService keystoreService, + ApplicationProperties applicationProperties) { + this.signingKeyRepository = signingKeyRepository; + this.keystoreService = keystoreService; + this.jwtProperties = applicationProperties.getSecurity().getJwt(); + } + + @Transactional + @Scheduled(fixedDelay = 1, timeUnit = TimeUnit.MINUTES) + public void cleanup() { + if (!jwtProperties.isEnableKeyCleanup() || !keystoreService.isKeystoreEnabled()) { + log.debug("Key cleanup is disabled, skipping cleanup"); + return; + } + + log.info("Removing inactive keys older than {} days", jwtProperties.getKeyRetentionDays()); + + try { + LocalDateTime cutoffDate = + LocalDateTime.now().minusDays(jwtProperties.getKeyRetentionDays()); + long totalKeysEligible = signingKeyRepository.countKeysEligibleForCleanup(cutoffDate); + + if (totalKeysEligible == 0) { + log.info("No keys eligible for cleanup"); + return; + } + + log.info("{} eligible keys found", totalKeysEligible); + + batchCleanup(cutoffDate); + } catch (Exception e) { + log.error("Error during scheduled key cleanup", e); + } + } + + private void batchCleanup(LocalDateTime cutoffDate) { + int batchSize = jwtProperties.getCleanupBatchSize(); + + while (true) { + Pageable pageable = PageRequest.of(0, batchSize); + List keysToCleanup = + signingKeyRepository.findInactiveKeysOlderThan(cutoffDate, pageable); + + if (keysToCleanup.isEmpty()) { + break; + } + + cleanupKeyBatch(keysToCleanup); + + if (keysToCleanup.size() < batchSize) { + break; + } + } + } + + private void cleanupKeyBatch(List keys) { + keys.forEach( + key -> { + try { + removePrivateKey(key.getKeyId()); + } catch (IOException e) { + log.warn("Failed to cleanup private key for keyId: {}", key.getKeyId(), e); + } + }); + + List keyIds = keys.stream().map(JwtSigningKey::getId).collect(Collectors.toList()); + + signingKeyRepository.deleteAllByIdInBatch(keyIds); + log.debug("Deleted {} signing keys from database", keyIds.size()); + } + + private void removePrivateKey(String keyId) throws IOException { + if (!keystoreService.isKeystoreEnabled()) { + return; + } + + Path privateKeyDirectory = Paths.get(InstallationPathConfig.getPrivateKeyPath()); + Path keyFile = privateKeyDirectory.resolve(keyId + JwtKeystoreService.KEY_SUFFIX); + + if (Files.exists(keyFile)) { + Files.delete(keyFile); + log.debug("Deleted private key file: {}", keyFile); + } else { + log.debug("Private key file not found: {}", keyFile); + } + } + + public long getKeysEligibleForCleanup() { + if (!jwtProperties.isEnableKeyCleanup() || !keystoreService.isKeystoreEnabled()) { + return 0; + } + + LocalDateTime cutoffDate = + LocalDateTime.now().minusDays(jwtProperties.getKeyRetentionDays()); + return signingKeyRepository.countKeysEligibleForCleanup(cutoffDate); + } +} diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtKeystoreService.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtKeystoreService.java index 72fbaba92..d1d3279bc 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtKeystoreService.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtKeystoreService.java @@ -20,7 +20,6 @@ import java.util.Optional; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; -import org.springframework.transaction.annotation.Transactional; import jakarta.annotation.PostConstruct; @@ -31,14 +30,13 @@ import stirling.software.common.model.ApplicationProperties; import stirling.software.proprietary.security.database.repository.JwtSigningKeyRepository; import stirling.software.proprietary.security.model.JwtSigningKey; -@Service @Slf4j +@Service public class JwtKeystoreService implements JwtKeystoreServiceInterface { public static final String KEY_SUFFIX = ".key"; private final JwtSigningKeyRepository repository; private final ApplicationProperties.Security.Jwt jwtProperties; - private final Path privateKeyDirectory; private volatile KeyPair currentKeyPair; private volatile String currentKeyId; @@ -48,13 +46,12 @@ public class JwtKeystoreService implements JwtKeystoreServiceInterface { JwtSigningKeyRepository repository, ApplicationProperties applicationProperties) { this.repository = repository; this.jwtProperties = applicationProperties.getSecurity().getJwt(); - this.privateKeyDirectory = Paths.get(InstallationPathConfig.getConfigPath(), "jwt-keys"); } @PostConstruct public void initializeKeystore() { if (!isKeystoreEnabled()) { - log.info("JWT keystore is disabled, using in-memory key generation"); + log.info("Keystore is disabled, using in-memory key generation"); return; } @@ -62,7 +59,7 @@ public class JwtKeystoreService implements JwtKeystoreServiceInterface { ensurePrivateKeyDirectoryExists(); loadOrGenerateKeypair(); } catch (Exception e) { - log.error("Failed to initialize JWT keystore, falling back to in-memory generation", e); + log.error("Failed to initialize keystore, falling back to in-memory generation", e); } } @@ -101,31 +98,6 @@ public class JwtKeystoreService implements JwtKeystoreServiceInterface { return currentKeyId; } - @Override - @Transactional - public void rotateKeypair() { - if (!isKeystoreEnabled()) { - log.warn("Cannot rotate keypair when keystore is disabled"); - return; - } - - try { - repository - .findByIsActiveTrue() - .ifPresent( - key -> { - key.setIsActive(false); - repository.save(key); - }); - - generateAndStoreKeypair(); - log.info("Successfully rotated JWT keypair"); - } catch (Exception e) { - log.error("Failed to rotate JWT keypair", e); - throw new RuntimeException("Keypair rotation failed", e); - } - } - @Override public boolean isKeystoreEnabled() { return jwtProperties.isEnableKeystore(); @@ -140,7 +112,7 @@ public class JwtKeystoreService implements JwtKeystoreServiceInterface { PrivateKey privateKey = loadPrivateKey(currentKeyId); PublicKey publicKey = decodePublicKey(activeKey.get().getSigningKey()); currentKeyPair = new KeyPair(publicKey, privateKey); - log.info("Loaded existing JWT keypair with keyId: {}", currentKeyId); + log.info("Loaded existing keypair with keyId: {}", currentKeyId); } catch (Exception e) { log.error("Failed to load existing keypair, generating new one", e); generateAndStoreKeypair(); @@ -163,7 +135,7 @@ public class JwtKeystoreService implements JwtKeystoreServiceInterface { currentKeyPair = keyPair; currentKeyId = keyId; - log.info("Generated and stored new JWT keypair with keyId: {}", keyId); + log.info("Generated and stored new keypair with keyId: {}", keyId); } catch (Exception e) { log.error("Failed to generate and store keypair", e); throw new RuntimeException("Keypair generation failed", e); @@ -189,14 +161,16 @@ public class JwtKeystoreService implements JwtKeystoreServiceInterface { } private void ensurePrivateKeyDirectoryExists() throws IOException { - if (!Files.exists(privateKeyDirectory)) { - Files.createDirectories(privateKeyDirectory); - log.info("Created JWT private key directory: {}", privateKeyDirectory); + Path keyPath = Paths.get(InstallationPathConfig.getPrivateKeyPath()); + + if (!Files.exists(keyPath)) { + Files.createDirectories(keyPath); } } private void storePrivateKey(String keyId, PrivateKey privateKey) throws IOException { - Path keyFile = privateKeyDirectory.resolve(keyId + KEY_SUFFIX); + Path keyFile = + Paths.get(InstallationPathConfig.getPrivateKeyPath()).resolve(keyId + KEY_SUFFIX); String encodedKey = Base64.getEncoder().encodeToString(privateKey.getEncoded()); Files.writeString(keyFile, encodedKey); @@ -212,9 +186,11 @@ public class JwtKeystoreService implements JwtKeystoreServiceInterface { private PrivateKey loadPrivateKey(String keyId) throws IOException, NoSuchAlgorithmException, InvalidKeySpecException { - Path keyFile = privateKeyDirectory.resolve(keyId + KEY_SUFFIX); + Path keyFile = + Paths.get(InstallationPathConfig.getPrivateKeyPath()).resolve(keyId + KEY_SUFFIX); + if (!Files.exists(keyFile)) { - throw new IOException("Private key file not found: " + keyFile); + throw new IOException("Private key not found: " + keyFile); } String encodedKey = Files.readString(keyFile); diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtKeystoreServiceInterface.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtKeystoreServiceInterface.java index 4df7d552d..dfb341b28 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtKeystoreServiceInterface.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtKeystoreServiceInterface.java @@ -11,7 +11,5 @@ public interface JwtKeystoreServiceInterface { String getActiveKeyId(); - void rotateKeypair(); - boolean isKeystoreEnabled(); } diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/security/service/JwtKeyCleanupServiceTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/security/service/JwtKeyCleanupServiceTest.java new file mode 100644 index 000000000..2483fc69b --- /dev/null +++ b/app/proprietary/src/test/java/stirling/software/proprietary/security/service/JwtKeyCleanupServiceTest.java @@ -0,0 +1,248 @@ +package stirling.software.proprietary.security.service; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.LocalDateTime; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.data.domain.Pageable; + +import stirling.software.common.configuration.InstallationPathConfig; +import stirling.software.common.model.ApplicationProperties; +import stirling.software.proprietary.security.database.repository.JwtSigningKeyRepository; +import stirling.software.proprietary.security.model.JwtSigningKey; + +@ExtendWith(MockitoExtension.class) +class JwtKeyCleanupServiceTest { + + @Mock + private JwtSigningKeyRepository signingKeyRepository; + + @Mock + private JwtKeystoreService keystoreService; + + @Mock + private ApplicationProperties applicationProperties; + + @Mock + private ApplicationProperties.Security security; + + @Mock + private ApplicationProperties.Security.Jwt jwtConfig; + + @TempDir + private Path tempDir; + + private JwtKeyCleanupService cleanupService; + + @BeforeEach + void setUp() { + lenient().when(applicationProperties.getSecurity()).thenReturn(security); + lenient().when(security.getJwt()).thenReturn(jwtConfig); + + lenient().when(jwtConfig.isEnableKeyCleanup()).thenReturn(true); + lenient().when(jwtConfig.getKeyRetentionDays()).thenReturn(7); + lenient().when(jwtConfig.getCleanupBatchSize()).thenReturn(100); + lenient().when(keystoreService.isKeystoreEnabled()).thenReturn(true); + + cleanupService = new JwtKeyCleanupService(signingKeyRepository, keystoreService, applicationProperties); + } + + + @Test + void testCleanupDisabled_ShouldSkip() { + when(jwtConfig.isEnableKeyCleanup()).thenReturn(false); + + cleanupService.cleanup(); + + verify(signingKeyRepository, never()).countKeysEligibleForCleanup(any(LocalDateTime.class)); + verify(signingKeyRepository, never()).findInactiveKeysOlderThan(any(LocalDateTime.class), any(Pageable.class)); + } + + @Test + void testCleanup_WhenKeystoreDisabled_ShouldSkip() { + when(keystoreService.isKeystoreEnabled()).thenReturn(false); + + cleanupService.cleanup(); + + verify(signingKeyRepository, never()).countKeysEligibleForCleanup(any(LocalDateTime.class)); + verify(signingKeyRepository, never()).findInactiveKeysOlderThan(any(LocalDateTime.class), any(Pageable.class)); + } + + @Test + void testCleanup_WhenNoKeysEligible_ShouldExitEarly() { + when(signingKeyRepository.countKeysEligibleForCleanup(any(LocalDateTime.class))).thenReturn(0L); + + cleanupService.cleanup(); + + verify(signingKeyRepository).countKeysEligibleForCleanup(any(LocalDateTime.class)); + verify(signingKeyRepository, never()).findInactiveKeysOlderThan(any(LocalDateTime.class), any(Pageable.class)); + } + + @Test + void testCleanupSuccessfully() throws IOException { + JwtSigningKey key1 = createTestKey("key-1", 1L); + JwtSigningKey key2 = createTestKey("key-2", 2L); + List keysToCleanup = Arrays.asList(key1, key2); + + try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { + mockedStatic.when(InstallationPathConfig::getPrivateKeyPath).thenReturn(tempDir.toString()); + + createTestKeyFile("key-1"); + createTestKeyFile("key-2"); + + when(signingKeyRepository.countKeysEligibleForCleanup(any(LocalDateTime.class))).thenReturn(2L); + when(signingKeyRepository.findInactiveKeysOlderThan(any(LocalDateTime.class), any(Pageable.class))) + .thenReturn(keysToCleanup) + .thenReturn(Collections.emptyList()); + + cleanupService.cleanup(); + + verify(signingKeyRepository).countKeysEligibleForCleanup(any(LocalDateTime.class)); + verify(signingKeyRepository).findInactiveKeysOlderThan(any(LocalDateTime.class), any(Pageable.class)); + verify(signingKeyRepository).deleteAllByIdInBatch(Arrays.asList(1L, 2L)); + + assertFalse(Files.exists(tempDir.resolve("key-1.key"))); + assertFalse(Files.exists(tempDir.resolve("key-2.key"))); + } + } + + @Test + void testCleanup_WithBatchProcessing_ShouldProcessMultipleBatches() throws IOException { + when(jwtConfig.getCleanupBatchSize()).thenReturn(2); + + JwtSigningKey key1 = createTestKey("key-1", 1L); + JwtSigningKey key2 = createTestKey("key-2", 2L); + JwtSigningKey key3 = createTestKey("key-3", 3L); + + List firstBatch = Arrays.asList(key1, key2); + List secondBatch = Arrays.asList(key3); + + try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { + mockedStatic.when(InstallationPathConfig::getPrivateKeyPath).thenReturn(tempDir.toString()); + + createTestKeyFile("key-1"); + createTestKeyFile("key-2"); + createTestKeyFile("key-3"); + + when(signingKeyRepository.countKeysEligibleForCleanup(any(LocalDateTime.class))).thenReturn(3L); + when(signingKeyRepository.findInactiveKeysOlderThan(any(LocalDateTime.class), any(Pageable.class))) + .thenReturn(firstBatch) + .thenReturn(secondBatch) + .thenReturn(Collections.emptyList()); + + cleanupService.cleanup(); + + verify(signingKeyRepository, times(2)).deleteAllByIdInBatch(any()); + verify(signingKeyRepository).deleteAllByIdInBatch(Arrays.asList(1L, 2L)); + verify(signingKeyRepository).deleteAllByIdInBatch(Arrays.asList(3L)); + } + } + + @Test + void testCleanup() throws IOException { + JwtSigningKey key1 = createTestKey("key-1", 1L); + JwtSigningKey key2 = createTestKey("key-2", 2L); + List keysToCleanup = Arrays.asList(key1, key2); + + try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { + mockedStatic.when(InstallationPathConfig::getPrivateKeyPath).thenReturn(tempDir.toString()); + + createTestKeyFile("key-1"); + + when(signingKeyRepository.countKeysEligibleForCleanup(any(LocalDateTime.class))).thenReturn(2L); + when(signingKeyRepository.findInactiveKeysOlderThan(any(LocalDateTime.class), any(Pageable.class))) + .thenReturn(keysToCleanup) + .thenReturn(Collections.emptyList()); + + cleanupService.cleanup(); + + verify(signingKeyRepository).deleteAllByIdInBatch(Arrays.asList(1L, 2L)); + assertFalse(Files.exists(tempDir.resolve("key-1.key"))); + } + } + + @Test + void testGetKeysEligibleForCleanup() { + when(signingKeyRepository.countKeysEligibleForCleanup(any(LocalDateTime.class))).thenReturn(5L); + + long result = cleanupService.getKeysEligibleForCleanup(); + + assertEquals(5L, result); + verify(signingKeyRepository).countKeysEligibleForCleanup(any(LocalDateTime.class)); + } + + @Test + void shouldReturnZero_WhenCleanupDisabled() { + when(jwtConfig.isEnableKeyCleanup()).thenReturn(false); + + long result = cleanupService.getKeysEligibleForCleanup(); + + assertEquals(0L, result); + verify(signingKeyRepository, never()).countKeysEligibleForCleanup(any(LocalDateTime.class)); + } + + @Test + void shouldReturnZero_WhenKeystoreDisabled() { + when(keystoreService.isKeystoreEnabled()).thenReturn(false); + + long result = cleanupService.getKeysEligibleForCleanup(); + + assertEquals(0L, result); + verify(signingKeyRepository, never()).countKeysEligibleForCleanup(any(LocalDateTime.class)); + } + + @Test + void testCleanup_WithRetentionDaysConfiguration_ShouldUseCorrectCutoffDate() { + when(jwtConfig.getKeyRetentionDays()).thenReturn(14); + when(signingKeyRepository.countKeysEligibleForCleanup(any(LocalDateTime.class))).thenReturn(0L); + + cleanupService.cleanup(); + + verify(signingKeyRepository).countKeysEligibleForCleanup(argThat((LocalDateTime cutoffDate) -> { + LocalDateTime expectedCutoff = LocalDateTime.now().minusDays(14); + return Math.abs(java.time.Duration.between(cutoffDate, expectedCutoff).toMinutes()) <= 1; + })); + } + + @Test + void testCleanupPrivateKeyFile_WhenKeystoreDisabled_ShouldSkipFileRemove() throws IOException { + when(keystoreService.isKeystoreEnabled()).thenReturn(false); + + cleanupService.cleanup(); + + verify(signingKeyRepository, never()).countKeysEligibleForCleanup(any(LocalDateTime.class)); + verify(signingKeyRepository, never()).findInactiveKeysOlderThan(any(LocalDateTime.class), any(Pageable.class)); + verify(signingKeyRepository, never()).deleteAllByIdInBatch(any()); + } + + private JwtSigningKey createTestKey(String keyId, Long id) { + JwtSigningKey key = new JwtSigningKey(); + key.setId(id); + key.setKeyId(keyId); + key.setSigningKey("test-public-key"); + key.setAlgorithm("RS256"); + key.setIsActive(false); + key.setCreatedAt(LocalDateTime.now().minusDays(10)); + return key; + } + + private void createTestKeyFile(String keyId) throws IOException { + Path keyFile = tempDir.resolve(keyId + ".key"); + Files.writeString(keyFile, "test-private-key-content"); + } +} diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/security/service/JwtKeystoreServiceInterfaceTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/security/service/JwtKeystoreServiceInterfaceTest.java index 98e2b836d..595c4ebf8 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/security/service/JwtKeystoreServiceInterfaceTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/security/service/JwtKeystoreServiceInterfaceTest.java @@ -1,9 +1,5 @@ package stirling.software.proprietary.security.service; -import static org.junit.jupiter.api.Assertions.*; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.*; - import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -12,7 +8,6 @@ import java.security.KeyPairGenerator; import java.security.NoSuchAlgorithmException; import java.util.Base64; import java.util.Optional; - import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -22,11 +17,19 @@ import org.junit.jupiter.params.provider.ValueSource; import org.mockito.Mock; import org.mockito.MockedStatic; import org.mockito.junit.jupiter.MockitoExtension; - import stirling.software.common.configuration.InstallationPathConfig; import stirling.software.common.model.ApplicationProperties; import stirling.software.proprietary.security.database.repository.JwtSigningKeyRepository; import stirling.software.proprietary.security.model.JwtSigningKey; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) class JwtKeystoreServiceInterfaceTest { @@ -55,9 +58,9 @@ class JwtKeystoreServiceInterfaceTest { keyPairGenerator.initialize(2048); testKeyPair = keyPairGenerator.generateKeyPair(); - when(applicationProperties.getSecurity()).thenReturn(security); - when(security.getJwt()).thenReturn(jwtConfig); - when(jwtConfig.isEnableKeystore()).thenReturn(true); + lenient().when(applicationProperties.getSecurity()).thenReturn(security); + lenient().when(security.getJwt()).thenReturn(jwtConfig); + lenient().when(jwtConfig.isEnableKeystore()).thenReturn(true); } @ParameterizedTest @@ -66,7 +69,7 @@ class JwtKeystoreServiceInterfaceTest { when(jwtConfig.isEnableKeystore()).thenReturn(keystoreEnabled); try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { - mockedStatic.when(InstallationPathConfig::getConfigPath).thenReturn(tempDir.toString()); + mockedStatic.when(InstallationPathConfig::getPrivateKeyPath).thenReturn(tempDir.toString()); keystoreService = new JwtKeystoreService(repository, applicationProperties); assertEquals(keystoreEnabled, keystoreService.isKeystoreEnabled()); @@ -78,7 +81,7 @@ class JwtKeystoreServiceInterfaceTest { when(jwtConfig.isEnableKeystore()).thenReturn(false); try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { - mockedStatic.when(InstallationPathConfig::getConfigPath).thenReturn(tempDir.toString()); + mockedStatic.when(InstallationPathConfig::getPrivateKeyPath).thenReturn(tempDir.toString()); keystoreService = new JwtKeystoreService(repository, applicationProperties); KeyPair result = keystoreService.getActiveKeypair(); @@ -94,7 +97,7 @@ class JwtKeystoreServiceInterfaceTest { when(repository.findByIsActiveTrue()).thenReturn(Optional.empty()); try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { - mockedStatic.when(InstallationPathConfig::getConfigPath).thenReturn(tempDir.toString()); + mockedStatic.when(InstallationPathConfig::getPrivateKeyPath).thenReturn(tempDir.toString()); keystoreService = new JwtKeystoreService(repository, applicationProperties); keystoreService.initializeKeystore(); @@ -114,12 +117,11 @@ class JwtKeystoreServiceInterfaceTest { JwtSigningKey existingKey = new JwtSigningKey(keyId, publicKeyBase64, "RS256"); when(repository.findByIsActiveTrue()).thenReturn(Optional.of(existingKey)); - Path keyFile = tempDir.resolve("jwt-keys").resolve(keyId + ".key"); - Files.createDirectories(keyFile.getParent()); + Path keyFile = tempDir.resolve(keyId + ".key"); Files.writeString(keyFile, privateKeyBase64); try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { - mockedStatic.when(InstallationPathConfig::getConfigPath).thenReturn(tempDir.toString()); + mockedStatic.when(InstallationPathConfig::getPrivateKeyPath).thenReturn(tempDir.toString()); keystoreService = new JwtKeystoreService(repository, applicationProperties); keystoreService.initializeKeystore(); @@ -139,12 +141,11 @@ class JwtKeystoreServiceInterfaceTest { JwtSigningKey signingKey = new JwtSigningKey(keyId, publicKeyBase64, "RS256"); when(repository.findByKeyId(keyId)).thenReturn(Optional.of(signingKey)); - Path keyFile = tempDir.resolve("jwt-keys").resolve(keyId + ".key"); - Files.createDirectories(keyFile.getParent()); + Path keyFile = tempDir.resolve(keyId + ".key"); Files.writeString(keyFile, privateKeyBase64); try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { - mockedStatic.when(InstallationPathConfig::getConfigPath).thenReturn(tempDir.toString()); + mockedStatic.when(InstallationPathConfig::getPrivateKeyPath).thenReturn(tempDir.toString()); keystoreService = new JwtKeystoreService(repository, applicationProperties); Optional result = keystoreService.getKeypairByKeyId(keyId); @@ -161,7 +162,7 @@ class JwtKeystoreServiceInterfaceTest { when(repository.findByKeyId(keyId)).thenReturn(Optional.empty()); try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { - mockedStatic.when(InstallationPathConfig::getConfigPath).thenReturn(tempDir.toString()); + mockedStatic.when(InstallationPathConfig::getPrivateKeyPath).thenReturn(tempDir.toString()); keystoreService = new JwtKeystoreService(repository, applicationProperties); Optional result = keystoreService.getKeypairByKeyId(keyId); @@ -175,7 +176,7 @@ class JwtKeystoreServiceInterfaceTest { when(jwtConfig.isEnableKeystore()).thenReturn(false); try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { - mockedStatic.when(InstallationPathConfig::getConfigPath).thenReturn(tempDir.toString()); + mockedStatic.when(InstallationPathConfig::getPrivateKeyPath).thenReturn(tempDir.toString()); keystoreService = new JwtKeystoreService(repository, applicationProperties); Optional result = keystoreService.getKeypairByKeyId("any-key"); @@ -184,54 +185,17 @@ class JwtKeystoreServiceInterfaceTest { } } - @Test - void testRotateKeypair() { - String oldKeyId = "old-key-123"; - JwtSigningKey oldKey = new JwtSigningKey(oldKeyId, "old-public-key", "RS256"); - when(repository.findByIsActiveTrue()).thenReturn(Optional.of(oldKey)); - - try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { - mockedStatic.when(InstallationPathConfig::getConfigPath).thenReturn(tempDir.toString()); - keystoreService = new JwtKeystoreService(repository, applicationProperties); - - keystoreService.initializeKeystore(); - - keystoreService.rotateKeypair(); - - assertFalse(oldKey.getIsActive()); - verify(repository, atLeast(2)).save(any(JwtSigningKey.class)); // At least one for deactivation, one for new key - - assertNotNull(keystoreService.getActiveKeyId()); - assertNotEquals(oldKeyId, keystoreService.getActiveKeyId()); - } - } - - @Test - void testRotateKeypairWhenKeystoreDisabled() { - when(jwtConfig.isEnableKeystore()).thenReturn(false); - - try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { - mockedStatic.when(InstallationPathConfig::getConfigPath).thenReturn(tempDir.toString()); - keystoreService = new JwtKeystoreService(repository, applicationProperties); - - assertDoesNotThrow(() -> keystoreService.rotateKeypair()); - - verify(repository, never()).save(any()); - } - } - @Test void testInitializeKeystoreCreatesDirectory() throws IOException { when(repository.findByIsActiveTrue()).thenReturn(Optional.empty()); try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { - mockedStatic.when(InstallationPathConfig::getConfigPath).thenReturn(tempDir.toString()); + mockedStatic.when(InstallationPathConfig::getPrivateKeyPath).thenReturn(tempDir.toString()); keystoreService = new JwtKeystoreService(repository, applicationProperties); keystoreService.initializeKeystore(); - Path jwtKeysDir = tempDir.resolve("jwt-keys"); - assertTrue(Files.exists(jwtKeysDir)); - assertTrue(Files.isDirectory(jwtKeysDir)); + assertTrue(Files.exists(tempDir)); + assertTrue(Files.isDirectory(tempDir)); } } @@ -244,7 +208,7 @@ class JwtKeystoreServiceInterfaceTest { when(repository.findByIsActiveTrue()).thenReturn(Optional.of(existingKey)); try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { - mockedStatic.when(InstallationPathConfig::getConfigPath).thenReturn(tempDir.toString()); + mockedStatic.when(InstallationPathConfig::getPrivateKeyPath).thenReturn(tempDir.toString()); keystoreService = new JwtKeystoreService(repository, applicationProperties); keystoreService.initializeKeystore();