[Destinations] Improve Detection and Masking of Secrets in Debug Logs (#798)

Co-authored-by: Alexander Dümont <alexander_duemont@web.de>

Author: MatKuhr

cloudplatform/cloudplatform-connectivity/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultDestination.java

@@ -7,6 +7,8 @@
 import java.util.Map;
 import java.util.TreeMap;
 import java.util.function.Function;
+import java.util.function.Predicate;
+import java.util.stream.Stream;
 
 import javax.annotation.Nonnull;
 
@@ -63,9 +65,11 @@ public Iterable<String> getPropertyNames()
     public String toString()
     {
         final Map<String, Object> nonSensitiveProperties = Maps.newHashMap(properties);
+        final Predicate<String> sensitivePropertyFilter =
+            key -> Stream.of("password", "secret").anyMatch(key::contains);
 
         for( final Map.Entry<String, Object> entry : nonSensitiveProperties.entrySet() ) {
-            if( entry.getKey().toLowerCase(Locale.ENGLISH).contains("password") ) {
+            if( sensitivePropertyFilter.test(entry.getKey().toLowerCase(Locale.ENGLISH)) ) {
                 entry.setValue("(hidden)");
             }
         }

cloudplatform/cloudplatform-connectivity/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultDestinationTest.java

@@ -43,8 +43,20 @@ void testIdentity()
     @Test
     void testToString()
     {
-        final Destination dest = DefaultDestination.builder().name("foo").property("some_password", "bar").build();
-        assertThat(dest).hasToString("DefaultDestination(properties={some_password=(hidden), Name=foo})");
+        final Destination dest =
+            DefaultDestination
+                .builder()
+                .name("foo")
+                .property("some_password", "bar")
+                .property("some_secret", "baz")
+                .build();
+
+        assertThat(dest.toString())
+            .startsWith("DefaultDestination(properties={")
+            .contains("some_password=(hidden)")
+            .contains("some_secret=(hidden)")
+            .contains("Name=foo")
+            .endsWith("})");
     }
 
     @Test

cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceAdapter.java

@@ -18,7 +18,6 @@
 import org.apache.http.StatusLine;
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.methods.HttpUriRequest;
-import org.slf4j.event.Level;
 
 import com.google.common.base.Strings;
 import com.sap.cloud.environment.servicebinding.api.DefaultServiceBindingAccessor;
@@ -146,37 +145,31 @@ private static String handleResponse( final HttpUriRequest request, final HttpRe
         final int statusCode = status.getStatusCode();
         final String reasonPhrase = status.getReasonPhrase();
 
+        log.debug("Destination service returned HTTP status {} ({})", statusCode, reasonPhrase);
+
         Try<String> maybeBody = Try.of(() -> HttpEntityUtil.getResponseBody(response));
-        String logMessage = "Destination service returned HTTP status %s (%s)";
         if( maybeBody.isFailure() ) {
             final var ex =
                 new DestinationAccessException("Failed to read body from HTTP response", maybeBody.getCause());
             maybeBody = Try.failure(ex);
-            logMessage = String.format(logMessage, statusCode, reasonPhrase);
-        } else {
-            logMessage = String.format(logMessage + "and body '%s'", statusCode, reasonPhrase, maybeBody.get());
         }
 
         if( statusCode == HttpStatus.SC_OK ) {
             final var ex = new DestinationAccessException("Failed to get destinations: no body returned in response.");
             maybeBody = maybeBody.filter(it -> !Strings.isNullOrEmpty(it), () -> ex);
-            log.atLevel(maybeBody.isSuccess() ? Level.DEBUG : Level.ERROR).log(logMessage);
             return maybeBody.get();
         }
 
-        log.error(logMessage);
         final String requestUri = request.getURI().getPath();
         if( statusCode == HttpStatus.SC_NOT_FOUND ) {
             throw new DestinationNotFoundException(null, "Destination could not be found for path " + requestUri + ".");
         }
-        throw new DestinationAccessException(
-            String
-                .format(
-                    "Failed to get destinations: destination service returned HTTP status %s (%S) at '%s'.,",
-                    statusCode,
-                    reasonPhrase,
-                    requestUri));
-
+        final String message =
+            "Failed to get destinations: destination service responded with HTTP status %s (%S) at '%s'."
+                .formatted(statusCode, reasonPhrase, requestUri);
+        final String messageWithBody = message + " Body: %s".formatted(maybeBody.getOrElseGet(Throwable::getMessage));
+        log.error(messageWithBody);
+        throw new DestinationAccessException(message);
     }
 
     private HttpUriRequest prepareRequest( final String servicePath, final DestinationRetrievalStrategy strategy )

release_notes.md

@@ -16,7 +16,7 @@
 
 ### 📈 Improvements
 
-- 
+- Improve the detection and masking of secrets when logging data to debug.
 
 ### 🐛 Fixed Issues