fix: AuthToken Regression (#433)

Author: Johannes Schneider

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

@@ -56,6 +56,9 @@ static Destination fromDestinationServiceV1Response(
         // enable tenant id
         baseBuilder.property(DestinationProperty.TENANT_ID, getDestinationTenantId(onBehalfOf));
 
+        // store auth tokens, if any
+        storeAuthTokens(baseBuilder, response.getAuthTokens());
+
         // finalize RFC destination
         if( baseBuilder.get(DestinationProperty.TYPE).contains(DestinationType.RFC) ) {
             return handleRfcDestination(baseBuilder.build());
@@ -108,18 +111,6 @@ private static Destination handleHttpDestination(
 
         // enable auth tokens
         if( authTokens != null && !authTokens.isEmpty() ) {
-            final AuthenticationType authType =
-                builder.get(DestinationProperty.AUTH_TYPE).getOrElse(AuthenticationType.NO_AUTHENTICATION);
-            authTokens.forEach(t -> throwOnTokenError(builder.get(DestinationProperty.NAME).getOrNull(), t));
-            authTokens.forEach(t -> setExpirationTimestamp(t, authType));
-
-            // Note: it is important that the auth tokens are added as property here
-            // for the HttpClientCache we need to include them in the cache key
-            // since they may contain cookies for which we need to make sure they
-            // are not shared between different http clients
-            // we can't attach the tokens directly to the header provider,
-            // because the header providers are excluded from the cache key
-            builder.property(DestinationProperty.AUTH_TOKENS, authTokens);
             builder.headerProviders(new AuthTokenHeaderProvider());
         }
 
@@ -140,6 +131,26 @@ private static void throwOnTokenError(
         }
     }
 
+    private static void storeAuthTokens(
+        @Nonnull final DefaultDestination.Builder builder,
+        @Nullable final List<DestinationServiceV1Response.DestinationAuthToken> authTokens )
+    {
+        if( authTokens == null || authTokens.isEmpty() ) {
+            return;
+        }
+
+        final AuthenticationType authType =
+            builder.get(DestinationProperty.AUTH_TYPE).getOrElse(AuthenticationType.NO_AUTHENTICATION);
+        authTokens.forEach(t -> throwOnTokenError(builder.get(DestinationProperty.NAME).getOrNull(), t));
+        authTokens.forEach(t -> setExpirationTimestamp(t, authType));
+
+        // Note: it is important that the auth tokens are added as property here
+        // for the HttpClientCache we need to include them in the cache key
+        // since they may contain cookies for which we need to make sure they
+        // are not shared between different http clients
+        builder.property(DestinationProperty.AUTH_TOKENS, authTokens);
+    }
+
     private static void setExpirationTimestamp(
         @Nonnull final DestinationServiceV1Response.DestinationAuthToken authToken,
         @Nonnull final AuthenticationType authType )

cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceFactoryTest.java

@@ -9,6 +9,7 @@
 import java.util.List;
 
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
 
 import com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceV1Response.DestinationAuthToken;
@@ -135,4 +136,35 @@ void propertyForwardAuthTokenShouldSetAuthenticationType()
 
         assertThat(result.asHttp().getAuthenticationType()).isEqualTo(AuthenticationType.TOKEN_FORWARDING);
     }
+
+    @Test
+    @DisplayName( "Regression: Auth Tokens are always stored" )
+    // see https://github.com/SAP/cloud-sdk-java/issues/428
+    void regressionTestTokenIsStoredForNonHttpDestination()
+    {
+        response.getDestinationConfiguration().put("Type", "MAIL");
+
+        final DestinationAuthToken authToken = new DestinationAuthToken();
+        response.setAuthTokens(List.of(authToken));
+
+        final Destination result = DestinationServiceFactory.fromDestinationServiceV1Response(response);
+
+        assertThat(result.get(DestinationProperty.AUTH_TOKENS)).containsExactly(List.of(authToken));
+    }
+
+    @Test
+    @DisplayName( "Regression: Auth Token error on non-HTTP destination leads to exception" )
+    // see https://github.com/SAP/cloud-sdk-java/issues/428
+    void regressionTestAuthTokenErrorLeadsToExceptionOnNonHttpDestination()
+    {
+        response.getDestinationConfiguration().put("Type", "MAIL");
+
+        final DestinationAuthToken authToken = new DestinationAuthToken();
+        authToken.setError("some error");
+        response.setAuthTokens(List.of(authToken));
+
+        assertThatThrownBy(() -> DestinationServiceFactory.fromDestinationServiceV1Response(response))
+            .isExactlyInstanceOf(DestinationAccessException.class)
+            .hasMessageContaining("some error");
+    }
 }

release_notes.md

@@ -32,3 +32,4 @@
 ### 🐛 Fixed Issues
 
 - Fix a regression that was introduced with the SAP Cloud SDK 5.0 release where the principal would no longer be derived from a `Basic` authorization header, in cases where neither a JWT nor an OIDC token was present.
+- Fix a regression that was introduced with the SAP Cloud SDK 5.0 release where auth tokens sent by the Destination service would no longer be stored in the `cloudsdk.authTokens` destination property for non-HTTP destinations.