diff --git a/packages/connectivity/src/scp-cf/destination/destination-from-env.ts b/packages/connectivity/src/scp-cf/destination/destination-from-env.ts index e2a2dbafe1..d9bfb341e9 100644 --- a/packages/connectivity/src/scp-cf/destination/destination-from-env.ts +++ b/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)) diff --git a/packages/connectivity/src/scp-cf/destination/destination-from-registration.ts b/packages/connectivity/src/scp-cf/destination/destination-from-registration.ts index 62058dad60..43d062c4fb 100644 --- a/packages/connectivity/src/scp-cf/destination/destination-from-registration.ts +++ b/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 { - 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)) diff --git a/packages/connectivity/src/scp-cf/destination/destination-from-service.ts b/packages/connectivity/src/scp-cf/destination/destination-from-service.ts index 25281e9e7d..a31019558d 100644 --- a/packages/connectivity/src/scp-cf/destination/destination-from-service.ts +++ b/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 { - // 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 { - 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 { - 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 { - 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 { - 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 { - 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 { - 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 { + 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 { @@ -429,12 +487,12 @@ Possible alternatives for such technical user authentication are BasicAuthentica } } - private async updateDestinationCache( + private async cacheDestination( destination: Destination, destinationOrigin: DestinationOrigin - ) { + ): Promise { 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 { - 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; } diff --git a/packages/connectivity/src/scp-cf/destination/destination-from-vcap.ts b/packages/connectivity/src/scp-cf/destination/destination-from-vcap.ts index 6549608b59..3dc305f37c 100644 --- a/packages/connectivity/src/scp-cf/destination/destination-from-vcap.ts +++ b/packages/connectivity/src/scp-cf/destination/destination-from-vcap.ts @@ -7,7 +7,7 @@ import { } from './http-proxy-util'; import { isHttpDestination } from './destination-service-types'; import { serviceToDestinationTransformers } from './service-binding-to-destination'; -import { setForwardedAuthTokenIfNeeded } from './forward-auth-token'; +import { addForwardedAuthTokenIfNeeded } from './forward-auth-token'; import type { DestinationFetchOptions } from './destination-accessor-types'; import type { Destination } from './destination-service-types'; import type { CachingOptions } from '../cache'; @@ -40,20 +40,16 @@ export async function getDestinationFromServiceBinding( : undefined; const retrievalOptions = { ...options, jwt: decodedJwt }; - const destination = await retrieveDestination(retrievalOptions); + let destination = (await retrieveDestination( + retrievalOptions + )) as Destination; - const destWithProxy = - destination && - isHttpDestination(destination) && - ['internet', 'private-link'].includes(proxyStrategy(destination)) - ? addProxyConfigurationInternet(destination) - : destination; - - if (destWithProxy) { - setForwardedAuthTokenIfNeeded(destWithProxy, options.jwt); - } + destination = addForwardedAuthTokenIfNeeded(destination, options.jwt); - return destWithProxy; + return isHttpDestination(destination) && + ['internet', 'private-link'].includes(proxyStrategy(destination)) + ? addProxyConfigurationInternet(destination) + : destination; } async function retrieveDestination({ diff --git a/packages/connectivity/src/scp-cf/destination/forward-auth-token.spec.ts b/packages/connectivity/src/scp-cf/destination/forward-auth-token.spec.ts index 9dd8c23677..7458c9be72 100644 --- a/packages/connectivity/src/scp-cf/destination/forward-auth-token.spec.ts +++ b/packages/connectivity/src/scp-cf/destination/forward-auth-token.spec.ts @@ -1,20 +1,20 @@ import { createLogger } from '@sap-cloud-sdk/util'; import { signedJwt } from '../../../../../test-resources/test/test-util'; -import { setForwardedAuthTokenIfNeeded } from './forward-auth-token'; +import { addForwardedAuthTokenIfNeeded } from './forward-auth-token'; +import type { Destination } from './destination-service-types'; describe('forward auth token', () => { it('sets an auth token, when `forwardAuthToken` is set', () => { const jwt = signedJwt({}); - const destination = setForwardedAuthTokenIfNeeded( - { forwardAuthToken: true }, - jwt - ); + let destination: Destination = { forwardAuthToken: true }; + destination = addForwardedAuthTokenIfNeeded(destination, jwt); expect(destination.authTokens?.[0]).toMatchObject({ value: jwt }); }); it('does not set an auth token, when `forwardAuthToken` is not set', () => { const jwt = signedJwt({}); - const destination = setForwardedAuthTokenIfNeeded({}, jwt); + let destination: Destination = {}; + destination = addForwardedAuthTokenIfNeeded({}, jwt); expect(destination.authTokens).toBeUndefined(); }); @@ -24,7 +24,7 @@ describe('forward auth token', () => { }); const warnSpy = jest.spyOn(logger, 'warn'); - setForwardedAuthTokenIfNeeded({ forwardAuthToken: true }); + addForwardedAuthTokenIfNeeded({ forwardAuthToken: true }); expect(warnSpy).toHaveBeenCalledWith( "Option 'forwardAuthToken' was set on destination but no token was provided to forward. This is most likely unintended and will lead to an authorization error on request execution." ); diff --git a/packages/connectivity/src/scp-cf/destination/forward-auth-token.ts b/packages/connectivity/src/scp-cf/destination/forward-auth-token.ts index c6e1920772..234acfd1dc 100644 --- a/packages/connectivity/src/scp-cf/destination/forward-auth-token.ts +++ b/packages/connectivity/src/scp-cf/destination/forward-auth-token.ts @@ -42,21 +42,26 @@ function validateToken(token: string | undefined): token is string { /** * @internal - * Set forwarded auth token, if needed. - * @param destination - Destination to set the token on, if needed. + * Add forwarded auth token, if needed. + * @param destination - Destination to add the token on, if needed. * @param token - Token to forward, if needed. */ -export function setForwardedAuthTokenIfNeeded( +export function addForwardedAuthTokenIfNeeded( destination: Destination, token?: string ): Destination { if (destination.forwardAuthToken) { + let authTokens: [DestinationAuthToken] | undefined; if (validateToken(token)) { logger.debug( "Option 'forwardAuthToken' enabled on destination. Using the given token for the destination." ); - destination.authTokens = buildDestinationAuthToken(token); + authTokens = buildDestinationAuthToken(token); } + return { + ...destination, + authTokens + }; } return destination; }