fix: Caching wrong auth token when fetching destination from provider account

packages/connectivity/src/scp-cf/destination/destination-from-env.ts

@@ -9,7 +9,7 @@ import {
   proxyStrategy
 } from './http-proxy-util';
 import { isHttpDestination } from './destination-service-types';
-import { setForwardedAuthTokenIfNeeded } from './forward-auth-token';
+import { addForwardedAuthTokenIfNeeded } from './forward-auth-token';
 import type { Destination } from './destination-service-types';
 import type { DestinationFetchOptions } from './destination-accessor-types';
 
@@ -103,13 +103,13 @@ export function searchEnvVariablesForDestination(
 
   if (getDestinationsEnvVariable()) {
     try {
-      const destination = getDestinationFromEnvByName(options.destinationName);
+      let destination = getDestinationFromEnvByName(options.destinationName);
       if (destination) {
         logger.info(
           `Successfully retrieved destination '${options.destinationName}' from environment variable.`
         );
 
-        setForwardedAuthTokenIfNeeded(destination, options.jwt);
+        destination = addForwardedAuthTokenIfNeeded(destination, options.jwt);
 
         return isHttpDestination(destination) &&
           ['internet', 'private-link'].includes(proxyStrategy(destination))

packages/connectivity/src/scp-cf/destination/destination-from-registration.ts

@@ -12,7 +12,7 @@ import {
   proxyStrategy
 } from './http-proxy-util';
 import { registerDestinationCache } from './register-destination-cache';
-import { setForwardedAuthTokenIfNeeded } from './forward-auth-token';
+import { addForwardedAuthTokenIfNeeded } from './forward-auth-token';
 import type { Destination } from './destination-service-types';
 import type { IsolationStrategy } from './destination-cache';
 import type { DestinationFetchOptions } from './destination-accessor-types';
@@ -90,7 +90,7 @@ export type DestinationWithName = Destination & { name: string };
 export async function searchRegisteredDestination(
   options: DestinationFetchOptions
 ): Promise<Destination | null> {
-  const destination =
+  let destination =
     await registerDestinationCache.destination.retrieveDestinationFromCache(
       getJwtForCaching(options),
       options.destinationName,
@@ -108,7 +108,7 @@ export async function searchRegisteredDestination(
     `Successfully retrieved destination '${options.destinationName}' from registered destinations.`
   );
 
-  setForwardedAuthTokenIfNeeded(destination, options.jwt);
+  destination = addForwardedAuthTokenIfNeeded(destination, options.jwt);
 
   return isHttpDestination(destination) &&
     ['internet', 'private-link'].includes(proxyStrategy(destination))

packages/connectivity/src/scp-cf/destination/destination-from-service.ts

@@ -33,7 +33,7 @@ import {
   addProxyConfigurationInternet,
   proxyStrategy
 } from './http-proxy-util';
-import { setForwardedAuthTokenIfNeeded } from './forward-auth-token';
+import { addForwardedAuthTokenIfNeeded } from './forward-auth-token';
 import type { SubscriberToken } from './get-subscriber-token';
 import type { Destination } from './destination-service-types';
 import type { AuthAndExchangeTokens } from './destination-service';
@@ -89,80 +89,65 @@ export class DestinationFromServiceRetriever {
   public static async getDestinationFromDestinationService(
     options: DestinationFetchOptions
   ): Promise<Destination | null> {
-    // TODO: This is currently always skipped for tokens issued by XSUAA
-    // in the XSUAA case no exchange takes place
-    if (shouldExchangeToken(options) && options.jwt) {
-      // Exchange the IAS token to a XSUAA token using the destination service credentials
-      options.jwt = await jwtBearerToken(options.jwt, 'destination');
+    // Exchange the IAS token to a XSUAA token using the destination service credentials
+    if (shouldExchangeToken(options)) {
+      options.jwt = await jwtBearerToken(options.jwt!, 'destination');
     }
 
-    const subscriberToken = await getSubscriberToken(options);
-    const providerToken = await getProviderServiceToken(options);
-
-    const da = new DestinationFromServiceRetriever(
+    // Create retriever with subscriber and provider tokens
+    const retriever = new DestinationFromServiceRetriever(
       options,
-      subscriberToken,
-      providerToken
+      await getSubscriberToken(options),
+      await getProviderServiceToken(options)
     );
 
-    const destinationResult =
-      await da.searchDestinationWithSelectionStrategyAndCache();
-    if (!destinationResult) {
+    // Search destination with selection strategy and cache
+    const destinationSearchResult =
+      await retriever.searchDestinationWithSelectionStrategyAndCache();
+
+    // Immediately return null if no destination found
+    if (!destinationSearchResult) {
       return null;
     }
 
-    let { destination } = destinationResult;
+    const { origin, fromCache } = destinationSearchResult;
+    let { destination } = destinationSearchResult;
 
-    setForwardedAuthTokenIfNeeded(destination, options.jwt);
+    if (!fromCache) {
+      /* Destination NOT from cache */
 
-    if (destinationResult.fromCache) {
-      return da.addProxyConfiguration(destination);
-    }
+      // Fetch and add auth token if needed,
+      // meaning `forwardAuthToken` is `false`
+      // AND authentication is one of the supported types
 
-    if (!destination.forwardAuthToken) {
-      if (
-        destination.authentication === 'OAuth2UserTokenExchange' ||
-        destination.authentication === 'OAuth2JWTBearer' ||
-        destination.authentication === 'SAMLAssertion' ||
-        (destination.authentication === 'OAuth2SAMLBearerAssertion' &&
-          !da.usesSystemUser(destination))
-      ) {
-        destination =
-          await da.fetchDestinationWithUserExchangeFlows(destinationResult);
-      }
-
-      if (destination.authentication === 'PrincipalPropagation') {
-        if (!this.isUserJwt(da.subscriberToken)) {
-          DestinationFromServiceRetriever.throwUserTokenMissing(destination);
-        }
-      }
+      // TODO: Check if the auth token is bound to options.jwt which might change next time for caching.
+      destination = await retriever.fetchAndAddAuthTokenIfNeeded(
+        destination,
+        origin
+      );
 
-      if (
-        destination.authentication === 'OAuth2Password' ||
-        destination.authentication === 'ClientCertificateAuthentication' ||
-        destination.authentication === 'OAuth2ClientCredentials' ||
-        da.usesSystemUser(destination)
-      ) {
-        destination =
-          await da.fetchDestinationWithNonUserExchangeFlows(destinationResult);
-      }
+      // Add trust store configuration if needed,
+      // meaning `TrustStoreLocation` is defined
+      destination = await retriever.addTrustStoreConfigurationIfNeeded(
+        destination,
+        origin
+      );
 
-      if (destination.authentication === 'OAuth2RefreshToken') {
-        destination =
-          await da.fetchDestinationWithRefreshTokenFlow(destinationResult);
-      }
+      // Cache the destination
+      await retriever.cacheDestination(destination, origin);
     }
 
-    const withTrustStore = await da.addTrustStoreConfiguration(
-      destination,
-      destinationResult.origin
-    );
-    await da.updateDestinationCache(withTrustStore, destinationResult.origin);
+    // Add auth token based on the given `options.jwt` if needed
+    // meaning `forwardAuthToken` is `true`
+    destination = addForwardedAuthTokenIfNeeded(destination, options.jwt);
+
+    // Add proxy configuration based on the proxy strategy
+    destination = await retriever.addProxyConfiguration(destination);
 
-    return da.addProxyConfiguration(withTrustStore);
+    return destination;
   }
 
-  private static throwUserTokenMissing(destination) {
+  private static throwUserTokenMissing(destination: Destination) {
     throw Error(
       `No user token (JWT) has been provided. This is strictly necessary for '${destination.authentication}'.`
     );
@@ -247,9 +232,9 @@ export class DestinationFromServiceRetriever {
   }
 
   private async getAuthTokenForOAuth2ClientCredentials(
-    destinationResult: DestinationSearchResult
+    destination: Destination,
+    origin: DestinationOrigin
   ): Promise<AuthAndExchangeTokens> {
-    const { destination, origin } = destinationResult;
     // This covers the x-tenant case https://api.sap.com/api/SAP_CP_CF_Connectivity_Destination/resource
     const exchangeTenant = this.getExchangeTenant(destination);
     const authHeaderJwt =
@@ -285,9 +270,9 @@ Possible alternatives for such technical user authentication are BasicAuthentica
   }
 
   private async getAuthTokenForOAuth2UserBasedTokenExchanges(
-    destinationResult: DestinationSearchResult
+    destination: Destination,
+    origin: DestinationOrigin
   ): Promise<AuthAndExchangeTokens> {
-    const { destination, origin } = destinationResult;
     const { destinationName } = this.options;
     if (!DestinationFromServiceRetriever.isUserJwt(this.subscriberToken)) {
       throw DestinationFromServiceRetriever.throwUserTokenMissing(destination);
@@ -341,9 +326,9 @@ Possible alternatives for such technical user authentication are BasicAuthentica
   }
 
   private async getAuthTokenForOAuth2RefreshToken(
-    destinationResult: DestinationSearchResult
+    destination: Destination,
+    origin: DestinationOrigin
   ): Promise<AuthAndExchangeTokens> {
-    const { destination, origin } = destinationResult;
     const { refreshToken } = this.options;
     if (!refreshToken) {
       throw Error(
@@ -361,14 +346,18 @@ Possible alternatives for such technical user authentication are BasicAuthentica
    * @internal
    * This method calls the 'find destination by name' endpoint of the destination service using a client credentials grant.
    * For the find by name endpoint, the destination service will take care of OAuth flows and include the token in the destination.
-   * @param destinationResult - Result of the getDestinations call for which the exchange flow is triggered.
+   * @param destination - The destination for which the token should be fetched.
+   * @param origin - The origin of the destination, either 'subscriber' or 'provider'.
    * @returns Destination containing the auth token.
    */
   private async fetchDestinationWithNonUserExchangeFlows(
-    destinationResult: DestinationSearchResult
+    destination: Destination,
+    origin: DestinationOrigin
   ): Promise<Destination> {
-    const token =
-      await this.getAuthTokenForOAuth2ClientCredentials(destinationResult);
+    const token = await this.getAuthTokenForOAuth2ClientCredentials(
+      destination,
+      origin
+    );
 
     return fetchDestinationWithTokenRetrieval(
       getDestinationServiceCredentials().uri,
@@ -378,12 +367,13 @@ Possible alternatives for such technical user authentication are BasicAuthentica
   }
 
   private async fetchDestinationWithUserExchangeFlows(
-    destinationResult: DestinationSearchResult
+    destination: Destination,
+    origin: DestinationOrigin
   ): Promise<Destination> {
-    const token =
-      await this.getAuthTokenForOAuth2UserBasedTokenExchanges(
-        destinationResult
-      );
+    const token = await this.getAuthTokenForOAuth2UserBasedTokenExchanges(
+      destination,
+      origin
+    );
 
     return fetchDestinationWithTokenRetrieval(
       getDestinationServiceCredentials().uri,
@@ -393,10 +383,13 @@ Possible alternatives for such technical user authentication are BasicAuthentica
   }
 
   private async fetchDestinationWithRefreshTokenFlow(
-    destinationResult: DestinationSearchResult
+    destination: Destination,
+    origin: DestinationOrigin
   ): Promise<Destination> {
-    const token =
-      await this.getAuthTokenForOAuth2RefreshToken(destinationResult);
+    const token = await this.getAuthTokenForOAuth2RefreshToken(
+      destination,
+      origin
+    );
 
     return fetchDestinationWithTokenRetrieval(
       getDestinationServiceCredentials().uri,
@@ -405,6 +398,71 @@ Possible alternatives for such technical user authentication are BasicAuthentica
     );
   }
 
+  private async fetchAndAddAuthTokenIfNeeded(
+    destination: Destination,
+    origin: DestinationOrigin
+  ): Promise<Destination> {
+    const { forwardAuthToken, authentication } = destination;
+    if (forwardAuthToken) {
+      return destination;
+    }
+
+    if (
+      authentication === 'OAuth2UserTokenExchange' ||
+      authentication === 'OAuth2JWTBearer' ||
+      authentication === 'SAMLAssertion' ||
+      (authentication === 'OAuth2SAMLBearerAssertion' &&
+        !this.usesSystemUser(destination))
+    ) {
+      // VERY BAD...
+      // If origin is provider, next time subscriber jwt might change.
+      // -> It might be an invalid user jwt next time, and SDK won't throw as destination cached already.
+      // -> SDK will use auth token retrieved with the previous subscriber jwt for a different subscriber next time.
+      destination = await this.fetchDestinationWithUserExchangeFlows(
+        destination,
+        origin
+      );
+    } else if (
+      authentication === 'OAuth2Password' ||
+      authentication === 'ClientCertificateAuthentication' ||
+      authentication === 'OAuth2ClientCredentials' ||
+      this.usesSystemUser(destination)
+    ) {
+      // SOMETIMES BAD!
+      // If origin is provider
+      // -> Auth token can be cached in destination cache as subscriber jwt is not used.
+      // -> UNLESS for `OAuth2ClientCredentials`, if `x-tenant` is used with `tokenServiceURLType` set to `Common` (see `getExchangeTenant()`),
+      //   then the subdomain of the tenant is sent to destination service and jwt will be exchanged based on the templated token service url.
+      // If origin is subscriber
+      // -> Auth token can be cached in destination cache as destination is tenant-isolated.
+      destination = await this.fetchDestinationWithNonUserExchangeFlows(
+        destination,
+        origin
+      );
+    } else if (authentication === 'OAuth2RefreshToken') {
+      // OK!
+      // If origin is provider, provider jwt + refresh token is used.
+      // -> Auth token can be cached in destination cache as subscriber is not used.
+      // If origin is subscriber, subscriber jwt + refresh token is used.
+      // -> Auth token can be cached in destination cache as destination is tenant-isolated.
+      destination = await this.fetchDestinationWithRefreshTokenFlow(
+        destination,
+        origin
+      );
+    } else if (authentication === 'PrincipalPropagation') {
+      // BAD...
+      // If origin is provider, next time subscriber jwt might change
+      // -> It might be an invalid user jwt next time, and SDK won't throw as destination cached already.
+      if (!DestinationFromServiceRetriever.isUserJwt(this.subscriberToken)) {
+        DestinationFromServiceRetriever.throwUserTokenMissing(destination);
+      }
+    }
+    return destination;
+
+    // For BAD cases above, we need to isolate the cache additionally with the subscriber jwt (better than using user jwt directly).
+    // `getSubscriberToken()` can be called freely as it is calling `serviceToken()` which uses caching itself.
+  }
+
   private async addProxyConfiguration(
     destination: Destination
   ): Promise<Destination> {
@@ -429,12 +487,12 @@ Possible alternatives for such technical user authentication are BasicAuthentica
     }
   }
 
-  private async updateDestinationCache(
+  private async cacheDestination(
     destination: Destination,
     destinationOrigin: DestinationOrigin
-  ) {
+  ): Promise<void> {
     if (!this.options.useCache) {
-      return destination;
+      return;
     }
     await destinationCache.cacheRetrievedDestination(
       destinationOrigin === 'subscriber'
@@ -571,19 +629,24 @@ Possible alternatives for such technical user authentication are BasicAuthentica
     );
   }
 
-  private async addTrustStoreConfiguration(
+  private async addTrustStoreConfigurationIfNeeded(
     destination: Destination,
     origin: DestinationOrigin
   ): Promise<Destination> {
-    if (destination.originalProperties?.TrustStoreLocation) {
+    const trustStoreLocation =
+      destination.originalProperties?.TrustStoreLocation;
+    if (trustStoreLocation) {
       const trustStoreCertificate = await fetchCertificate(
         getDestinationServiceCredentials().uri,
         origin === 'provider'
           ? this.providerServiceToken.encoded
           : this.subscriberToken!.serviceJwt!.encoded,
-        destination.originalProperties.TrustStoreLocation
+        trustStoreLocation
       );
-      destination.trustStoreCertificate = trustStoreCertificate;
+      return {
+        ...destination,
+        trustStoreCertificate
+      };
     }
     return destination;
   }