diff --git a/modules/client/README.md b/modules/client/README.md index 82c034952..23483ae7a 100644 --- a/modules/client/README.md +++ b/modules/client/README.md @@ -125,6 +125,10 @@ The `firefly.service-client` namespace is bound by [ServiceClientProperties.java | [`GrpcClient`](https://github.com/fireflyframework/fireflyframework-jafly/blob/main/modules/client/src/main/java/org/fireflyframework/client/GrpcClient.java) | gRPC-specific service client interface with native gRPC semantics. | | [`ServiceClient`](https://github.com/fireflyframework/fireflyframework-jafly/blob/main/modules/client/src/main/java/org/fireflyframework/client/ServiceClient.java) | Core interface for all service clients providing common lifecycle and health operations. | +## Security diagnostics + +`SecretsEncryptionManager` reports storage, missing-secret, and removal outcomes without logging secret names. Successful `JwtValidator` validation does not log the JWT subject. These diagnostics remain available at their existing log levels. Keep credentials and customer identifiers out of application-supplied log messages as well. + ## Continue the application Use the [Orchestration guide](../../docs/site/modules/orchestration.md) for the next part of this integration. diff --git a/modules/client/src/main/java/org/fireflyframework/client/security/JwtValidator.java b/modules/client/src/main/java/org/fireflyframework/client/security/JwtValidator.java index 7ee81e3de..531dca8cd 100644 --- a/modules/client/src/main/java/org/fireflyframework/client/security/JwtValidator.java +++ b/modules/client/src/main/java/org/fireflyframework/client/security/JwtValidator.java @@ -135,7 +135,7 @@ public JwtClaims validate(String token) throws JwtValidationException { // Validate claims validateClaims(claims); - log.debug("JWT validated successfully for subject: {}", claims.getSubject()); + log.debug("JWT validated successfully"); return claims; } diff --git a/modules/client/src/main/java/org/fireflyframework/client/security/SecretsEncryptionManager.java b/modules/client/src/main/java/org/fireflyframework/client/security/SecretsEncryptionManager.java index 5b3473596..738d93133 100644 --- a/modules/client/src/main/java/org/fireflyframework/client/security/SecretsEncryptionManager.java +++ b/modules/client/src/main/java/org/fireflyframework/client/security/SecretsEncryptionManager.java @@ -180,7 +180,7 @@ public void storeSecret(String key, String value) { } String encrypted = encrypt(value); encryptedSecrets.put(key, encrypted); - log.debug("Stored encrypted secret: {}", key); + log.debug("Stored encrypted secret"); } /** @@ -192,7 +192,7 @@ public void storeSecret(String key, String value) { public String getSecret(String key) { String encrypted = encryptedSecrets.get(key); if (encrypted == null) { - log.warn("Secret not found: {}", key); + log.warn("Secret not found"); return null; } return decrypt(encrypted); @@ -207,7 +207,7 @@ public String getSecret(String key) { public boolean removeSecret(String key) { boolean removed = encryptedSecrets.remove(key) != null; if (removed) { - log.debug("Removed secret: {}", key); + log.debug("Removed secret"); } return removed; } diff --git a/modules/client/src/test/java/org/fireflyframework/client/security/SecurityLogPrivacyTest.java b/modules/client/src/test/java/org/fireflyframework/client/security/SecurityLogPrivacyTest.java new file mode 100644 index 000000000..026b93618 --- /dev/null +++ b/modules/client/src/test/java/org/fireflyframework/client/security/SecurityLogPrivacyTest.java @@ -0,0 +1,118 @@ +package org.fireflyframework.client.security; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; + +import javax.crypto.Mac; +import javax.crypto.spec.SecretKeySpec; +import java.nio.charset.StandardCharsets; +import java.time.Instant; +import java.util.Arrays; +import java.util.Base64; +import java.util.Map; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; + +class SecurityLogPrivacyTest { + @Test + void storingAndReadingASecretRetainsItsValueWithoutLoggingItsNameOrContent() { + var manager = manager(); + String name = "synthetic-private-credential-name"; + String value = UUID.randomUUID().toString(); + try (var capture = new LogCapture(SecretsEncryptionManager.class)) { + manager.storeSecret(name, value); + assertThat(manager.getSecret(name)).isEqualTo(value); + assertThat(manager.getSecretCount()).isEqualTo(1); + capture.assertExcludes(name, value); + } + } + + @Test + void removingASecretRetainsTheRemovalOutcomeWithoutLoggingItsName() { + var manager = manager(); + String name = "synthetic-private-removal-name"; + manager.storeSecret(name, UUID.randomUUID().toString()); + try (var capture = new LogCapture(SecretsEncryptionManager.class)) { + assertThat(manager.removeSecret(name)).isTrue(); + assertThat(manager.hasSecret(name)).isFalse(); + assertThat(manager.removeSecret(name)).isFalse(); + capture.assertExcludes(name); + } + } + + @Test + void aMissingSecretRetainsTheAbsentResultWithoutLoggingTheLookupName() { + var manager = manager(); + String name = "synthetic-private-missing-name"; + try (var capture = new LogCapture(SecretsEncryptionManager.class)) { + assertThat(manager.getSecret(name)).isNull(); + capture.assertExcludes(name); + } + } + + @Test + void successfulSignatureValidationRetainsClaimsWithoutLoggingTheSubjectOrToken() throws Exception { + String secret = UUID.randomUUID().toString(); + String subject = "synthetic-private-customer-subject"; + String issuer = "https://issuer.example.invalid"; + String audience = "synthetic-client"; + var mapper = new ObjectMapper(); + var encoder = Base64.getUrlEncoder().withoutPadding(); + String header = encoder.encodeToString(mapper.writeValueAsBytes(Map.of("alg", "HS256", "typ", "JWT"))); + String payload = encoder.encodeToString(mapper.writeValueAsBytes(Map.of( + "sub", subject, "iss", issuer, "aud", audience, + "exp", Instant.now().plusSeconds(300).getEpochSecond()))); + String unsigned = header + "." + payload; + Mac mac = Mac.getInstance("HmacSHA256"); + mac.init(new SecretKeySpec(secret.getBytes(StandardCharsets.UTF_8), "HmacSHA256")); + String token = unsigned + "." + encoder.encodeToString(mac.doFinal(unsigned.getBytes(StandardCharsets.UTF_8))); + var validator = JwtValidator.builder().secret(secret).issuer(issuer).audience(audience).build(); + try (var capture = new LogCapture(JwtValidator.class)) { + assertThat(validator.validate(token).getSubject()).isEqualTo(subject); + capture.assertExcludes(subject, token, secret); + } + } + + private static SecretsEncryptionManager manager() { + return SecretsEncryptionManager.builder() + .masterKey(UUID.randomUUID().toString().replace("-", "")) + .build(); + } + + private static final class LogCapture implements AutoCloseable { + private final Logger logger; + private final Level previousLevel; + private final ListAppender appender = new ListAppender<>(); + + private LogCapture(Class type) { + logger = (Logger) LoggerFactory.getLogger(type); + previousLevel = logger.getLevel(); + logger.setLevel(Level.DEBUG); + appender.start(); + logger.addAppender(appender); + } + + private void assertExcludes(String... values) { + assertThat(appender.list).isNotEmpty(); + for (var event : appender.list) { + String diagnostic = event.getFormattedMessage() + Arrays.deepToString(event.getArgumentArray()); + for (String value : values) { + assertThat(diagnostic).doesNotContain(value); + } + } + } + + @Override + public void close() { + logger.detachAppender(appender); + logger.setLevel(previousLevel); + appender.stop(); + } + } +}