Repository: Unleash/unleash-client-java
Pull request #207 (2023-08-07 09:33:51)

fix: Use sha256 digest instead of exposing api key
As mentioned in #206 - The way we did the identifier for a client inadvertently also exposes api keys in the logs when logging that you have more than one client using the same configuration. This fix uses the SHA-256 hash of the three elements (apiKey, appName and instanceId) together and then uses the hex representation of  that as the client identifier instead.

In addition this updates the log message to tell you that you have n clients for appName:instanceId combination.

sonatype-lift[bot]: ## Sonatype Lift is retiring
Sonatype Lift will be retiring on <b>Sep 12, 2023</b>, with its analysis stopping on Aug 12, 2023. We understand that this news may come as a disappointment, and Sonatype is committed to helping you transition off it seamlessly. If you’d like to retain your data, please export your issues from the web console.
We are extremely grateful and thank you for your support over the years.

<b>[📖 Read about the impacts and timeline](https://www.sonatype.com/lift-retirement)</b>

Patch:
diff --git a/src/main/java/io/getunleash/DefaultUnleash.java b/src/main/java/io/getunleash/DefaultUnleash.java
index 73f68525..f9a23364 100644
--- a/src/main/java/io/getunleash/DefaultUnleash.java
+++ b/src/main/java/io/getunleash/DefaultUnleash.java
@@ -100,14 +100,16 @@ public class DefaultUnleash implements Unleash {
         this.eventDispatcher = eventDispatcher;
         this.metricService = metricService;
         metricService.register(strategyMap.keySet());
-        this.initCounts.compute(
+        initCounts.compute(
                 config.getClientIdentifier(),
                 (key, inits) -> {
                     if (inits != null) {
                         String error =
                                 String.format(
-                                        "You already have %d clients for Unleash Configuration [%s] running. Please double check your code where you are instantiating the Unleash SDK",
-                                        inits.sum(), key);
+                                        "You already have %d clients for AppName [%s] with instanceId: [%s] running. Please double check your code where you are instantiating the Unleash SDK",
+                                        inits.sum(),
+                                        unleashConfig.getAppName(),
+                                        unleashConfig.getInstanceId());
                         if (failOnMultipleInstantiations) {
                             throw new RuntimeException(error);
                         } else {
diff --git a/src/main/java/io/getunleash/util/UnleashConfig.java b/src/main/java/io/getunleash/util/UnleashConfig.java
index 6f8e2cfc..1191df51 100644
--- a/src/main/java/io/getunleash/util/UnleashConfig.java
+++ b/src/main/java/io/getunleash/util/UnleashConfig.java
@@ -13,6 +13,7 @@ import io.getunleash.repository.HttpFeatureFetcher;
 import io.getunleash.repository.ToggleBootstrapProvider;
 import io.getunleash.strategy.Strategy;
 import java.io.File;
+import java.math.BigInteger;
 import java.net.Authenticator;
 import java.net.HttpURLConnection;
 import java.net.InetAddress;
@@ -21,10 +22,11 @@ import java.net.PasswordAuthentication;
 import java.net.Proxy;
 import java.net.URI;
 import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
 import java.time.Duration;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Optional;
+import java.util.*;
 
 public class UnleashConfig {
 
@@ -271,10 +273,17 @@ public class UnleashConfig {
     }
 
     public String getClientIdentifier() {
-        String api = getApiKey();
-        String appName = getAppName();
-        String instanceId = getInstanceId();
-        return String.format("apiKey:[%s] appName:[%s] instanceId:[%s]", api, appName, instanceId);
+        try {
+            MessageDigest md = MessageDigest.getInstance("SHA-256");
+            if (getApiKey() != null) {
+                md.update(getApiKey().getBytes(StandardCharsets.UTF_8));
+            }
+            md.update(getAppName().getBytes(StandardCharsets.UTF_8));
+            md.update(getInstanceId().getBytes(StandardCharsets.UTF_8));
+            return new BigInteger(1, md.digest()).toString(16);
+        } catch (NoSuchAlgorithmException nse) {
+            throw new IllegalStateException("Could not build hash for client", nse);
+        }
     }
 
     public boolean isSynchronousFetchOnInitialisation() {
@@ -420,7 +429,7 @@ public class UnleashConfig {
 
         private static String getHostname() {
             String hostName = System.getProperty("hostname");
-            if (hostName == null || hostName.length() == 0) {
+            if (hostName == null || hostName.isEmpty()) {
                 try {
                     hostName = InetAddress.getLocalHost().getHostName();
                 } catch (UnknownHostException e) {


Test patch:
diff --git a/src/test/java/io/getunleash/DefaultUnleashTest.java b/src/test/java/io/getunleash/DefaultUnleashTest.java
index f869429c..4f2205c8 100644
--- a/src/test/java/io/getunleash/DefaultUnleashTest.java
+++ b/src/test/java/io/getunleash/DefaultUnleashTest.java
@@ -161,12 +161,14 @@ class DefaultUnleashTest {
         appender.start();
         Logger unleashLogger = (Logger) LoggerFactory.getLogger(DefaultUnleash.class);
         unleashLogger.addAppender(appender);
+        String appName = "multiple_connection_logging";
+        String instanceId = "multiple_connection_instance_id";
         UnleashConfig config =
                 UnleashConfig.builder()
                         .unleashAPI("http://test:4242")
-                        .appName("multiple_connection_logging")
+                        .appName(appName)
                         .apiKey("default:development:1234567890123456")
-                        .instanceId("multiple_connection_logging")
+                        .instanceId(instanceId)
                         .build();
         Unleash unleash1 = new DefaultUnleash(config);
         // We've only instantiated the client once, so no errors should've been logged
@@ -178,8 +180,10 @@ class DefaultUnleashTest {
         assertThat(appender.list)
                 .extracting(ILoggingEvent::getFormattedMessage)
                 .containsExactly(
-                        "You already have 1 clients for Unleash Configuration ["
-                                + id
+                        "You already have 1 clients for AppName ["
+                                + appName
+                                + "] with instanceId: ["
+                                + instanceId
                                 + "] running. Please double check your code where you are instantiating the Unleash SDK");
         appender.list.clear();
         Unleash unleash3 = new DefaultUnleash(config);
@@ -188,8 +192,10 @@ class DefaultUnleashTest {
         assertThat(appender.list)
                 .extracting(ILoggingEvent::getFormattedMessage)
                 .containsExactly(
-                        "You already have 2 clients for Unleash Configuration ["
-                                + id
+                        "You already have 2 clients for AppName ["
+                                + appName
+                                + "] with instanceId: ["
+                                + instanceId
                                 + "] running. Please double check your code where you are instantiating the Unleash SDK");
     }
 
@@ -266,7 +272,6 @@ class DefaultUnleashTest {
                         .build();
         String id = config.getClientIdentifier();
         assertThat(id)
-                .isEqualTo(
-                        "apiKey:[null] appName:[multiple_connection] instanceId:[testing_multiple]");
+                .isEqualTo("f83eb743f4c8dc41294aafb96f454763e5a90b96db8b7040ddc505d636bdb243");
     }
 }
diff --git a/src/test/java/io/getunleash/util/UnleashConfigTest.java b/src/test/java/io/getunleash/util/UnleashConfigTest.java
index 7d5bd812..7ada84dc 100644
--- a/src/test/java/io/getunleash/util/UnleashConfigTest.java
+++ b/src/test/java/io/getunleash/util/UnleashConfigTest.java
@@ -3,6 +3,7 @@ package io.getunleash.util;
 import static io.getunleash.util.UnleashConfig.UNLEASH_APP_NAME_HEADER;
 import static io.getunleash.util.UnleashConfig.UNLEASH_INSTANCE_ID_HEADER;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.Mockito.when;
 
@@ -346,4 +347,38 @@ public class UnleashConfigTest {
         assertThat(new String(passwordAuthentication.getPassword())).isEqualTo(proxyPassword);
         assertThat(config.isProxyAuthenticationByJvmProperties()).isEqualTo(false);
     }
+
+    @Test
+    public void clientIdentifierIsDeterministic() {
+        UnleashConfig config =
+                UnleashConfig.builder()
+                        .apiKey("someapikey")
+                        .appName("test-app")
+                        .instanceId("myinstance")
+                        .unleashAPI("http://localhost:4242")
+                        .build();
+        assertThat(config.getClientIdentifier()).isEqualTo(config.getClientIdentifier());
+    }
+
+    @Test
+    public void clientIdentifierWithoutApiKeyShouldNotThrowException() {
+        UnleashConfig config =
+                UnleashConfig.builder()
+                        .appName("test-app")
+                        .instanceId("myiunstance")
+                        .unleashAPI("http://localhost:4242")
+                        .build();
+        assertDoesNotThrow(config::getClientIdentifier);
+    }
+
+    @Test
+    public void clientIdentifierWithoutInstanceIdStillWorks() {
+        UnleashConfig config =
+                UnleashConfig.builder()
+                        .apiKey("someapikey")
+                        .appName("test-app")
+                        .unleashAPI("http://localhost:4242")
+                        .build();
+        assertDoesNotThrow(config::getClientIdentifier);
+    }
 }