fix: [Destination] Fix Error Handling when Principal is Unavailable (#…

…202) Co-authored-by: Matthias Kuhr <[email protected]>
SAP · Dec 27, 2023 · dff2829 · dff2829
1 parent 09c866f
commit dff2829
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 20 deletions.
diff --git a/...va/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java b/...va/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java
@@ -170,7 +170,12 @@ Try<Destination> execute()
                     if( !DestinationUtility.requiresUserTokenExchange(result) ) {
                         destinationCache.put(cacheKey, result);
                     } else {
-                        throwIfPrincipalIsUnavailable(destinationName, exchangeStrategy);
+                        if( additionalKeyWithTenantAndPrincipal.getPrincipalId().isEmpty() ) {
+                            final String message =
+                                "No principal is available in the current ThreadContext, but a principal is required for fetching the destination %s. "
+                                    + "This typically means that the current context is malformed, containing inconsistent information about the authentication token, tenant and principal.";
+                            return Try.failure(new DestinationAccessException(message.formatted(destinationName)));
+                        }
                         destinationCache.put(additionalKeyWithTenantAndPrincipal, result);
                     }
                     break;
@@ -182,19 +187,6 @@ Try<Destination> execute()
         }
     }
 
-    private void throwIfPrincipalIsUnavailable(
-        @Nonnull final String destinationName,
-        @Nonnull final DestinationServiceTokenExchangeStrategy exchangeStrategy )
-    {
-        if( additionalKeyWithTenantAndPrincipal.getPrincipalId().isEmpty() ) {
-            throw new IllegalStateException(
-                "Principal ID is not available in the incoming request, but is required for fetching destination "
-                    + destinationName
-                    + " that requires user token exchange with strategy "
-                    + exchangeStrategy);
-        }
-    }
-
     @SuppressWarnings( "deprecation" )
     private void logErroneousCombinations(
         @Nonnull final DestinationProperties result,

diff --git a/...java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeDestinationCommandTest.java b/...java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeDestinationCommandTest.java
@@ -7,7 +7,6 @@
 import static com.sap.cloud.sdk.cloudplatform.connectivity.AuthenticationType.OAUTH2_CLIENT_CREDENTIALS;
 import static com.sap.cloud.sdk.cloudplatform.connectivity.AuthenticationType.OAUTH2_JWT_BEARER;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
@@ -413,7 +412,6 @@ void testForwardUserTokenStrategyForUserPropagationDestinationWithoutJwt()
         assertThat(fetchedDestination.getCause())
             .isInstanceOf(DestinationAccessException.class)
             .hasMessageContaining("principal-is-missing");
-
     }
 
     @Test
@@ -446,10 +444,10 @@ void testForwardUserTokenStrategyForUserPropagationDestinationWithoutPrincipalUn
                         .prepareCommand(DESTINATION_NAME, options, destinationCache, isolationLocks, function, null)
                         .get());
 
-        assertThatThrownBy(() -> TenantAccessor.executeWithTenant(t1, () -> sut.execute()))
-            .hasRootCauseInstanceOf(IllegalStateException.class)
-            .hasMessageContaining("Principal ID is not available in the incoming request");
-
+        final Try<Destination> shouldBeFailure = TenantAccessor.executeWithTenant(t1, sut::execute);
+        assertThat(shouldBeFailure.getCause())
+            .isInstanceOf(DestinationAccessException.class)
+            .hasMessageContaining("No principal is available in the current ThreadContext");
     }
 
     @Test