Skip to content

Commit

Permalink
322 - remove CURRENT_TENANT_THEN_PROVIDER (#86)
Browse files Browse the repository at this point in the history 
Co-authored-by: Alexander Dümont <[email protected]>
  • Loading branch information
@rnewbigging @newtork
rnewbigging and newtork authored Oct 6, 2023
1 parent 34c9fda commit d6c6664
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 385 deletions.
158 changes: 18 additions & 140 deletions ...va/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationRetrievalStrategyResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import static com.sap.cloud.sdk.cloudplatform.connectivity.OnBehalfOf.TECHNICAL_USER_PROVIDER;
import static com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationRetrievalStrategy.ALWAYS_PROVIDER;
import static com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationRetrievalStrategy.CURRENT_TENANT;
import static com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationRetrievalStrategy.CURRENT_TENANT_THEN_PROVIDER;
import static com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationRetrievalStrategy.ONLY_SUBSCRIBER;
import static com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationTokenExchangeStrategy.EXCHANGE_ONLY;
import static com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationTokenExchangeStrategy.FORWARD_USER_TOKEN;
Expand All @@ -29,7 +28,6 @@
import javax.annotation.Nullable;

import com.sap.cloud.sdk.cloudplatform.connectivity.exception.DestinationAccessException;
import com.sap.cloud.sdk.cloudplatform.connectivity.exception.DestinationNotFoundException;
import com.sap.cloud.sdk.cloudplatform.security.AuthTokenAccessor;
import com.sap.cloud.sdk.cloudplatform.tenant.Tenant;
import com.sap.cloud.sdk.cloudplatform.tenant.TenantAccessor;
Expand All @@ -41,13 +39,13 @@

@Slf4j
@RequiredArgsConstructor
@SuppressWarnings( { "PMD.TooManyStaticImports", "deprecation" } ) // without these static imports the code becomes unreadable
@SuppressWarnings( "PMD.TooManyStaticImports" ) // without these static imports the code becomes unreadable
class DestinationRetrievalStrategyResolver
{
private static final Strategy tokenExchangeOnlyStrategy = new Strategy(NAMED_USER_CURRENT_TENANT, false);
private final Supplier<String> providerTenantIdSupplier;
private final Function<Strategy, ScpCfDestinationServiceV1Response> destinationRetriever;
private final Function<OnBehalfOf, List<Destination>> allDstinationRetriever;
private final Function<OnBehalfOf, List<Destination>> allDestinationRetriever;

static final String JWT_ATTR_EXT = "ext_attr";
static final String JWT_ATTR_ENHANCER = "enhancer";
Expand All @@ -72,12 +70,12 @@ static DestinationRetrievalStrategyResolver forSingleDestination(

static DestinationRetrievalStrategyResolver forAllDestinations(
final Supplier<String> providerTenantIdSupplier,
final Function<OnBehalfOf, List<Destination>> allDstinationRetriever )
final Function<OnBehalfOf, List<Destination>> allDestinationRetriever )
{
return new DestinationRetrievalStrategyResolver(
providerTenantIdSupplier,
( any ) -> null,
allDstinationRetriever);
allDestinationRetriever);
}

Strategy resolveSingleRequestStrategy(
Expand All @@ -94,7 +92,7 @@ Strategy resolveSingleRequestStrategy(
case ONLY_SUBSCRIBER:
behalfTechnicalUser = TECHNICAL_USER_CURRENT_TENANT;
break;
// sanity check that this method is never called for CURRENT_TENANT_THEN_PROVIDER
// sanity check
default:
throw new IllegalStateException(
"Unexpected retrieval strategy "
Expand Down Expand Up @@ -164,34 +162,20 @@ private ScpCfDestinationTokenExchangeStrategy getDefaultTokenExchangeStrategy()
}

Supplier<ScpCfDestinationServiceV1Response> prepareSupplier(
@Nonnull final ScpCfDestinationRetrievalStrategy originalRetrievalStrategy,
@Nonnull final ScpCfDestinationRetrievalStrategy retrievalStrategy,
@Nonnull final ScpCfDestinationTokenExchangeStrategy tokenExchangeStrategy )
throws DestinationAccessException
{
log
.debug(
"Preparing request(s) towards the destination service based on the strategies {} and {}",
originalRetrievalStrategy,
retrievalStrategy,
tokenExchangeStrategy);
warnOrThrowOnDeprecatedOrUnsupportedCombinations(originalRetrievalStrategy, tokenExchangeStrategy);

final ScpCfDestinationRetrievalStrategy retrievalStrategy;
if( originalRetrievalStrategy == CURRENT_TENANT_THEN_PROVIDER && currentTenantIsProvider() ) {
retrievalStrategy = CURRENT_TENANT;
} else {
retrievalStrategy = originalRetrievalStrategy;
}
warnOrThrowOnDeprecatedOrUnsupportedCombinations(retrievalStrategy, tokenExchangeStrategy);

final Strategy strategy;

// handle the simple cases
if( tokenExchangeStrategy != LOOKUP_THEN_EXCHANGE && retrievalStrategy != CURRENT_TENANT_THEN_PROVIDER ) {
strategy = resolveSingleRequestStrategy(retrievalStrategy, tokenExchangeStrategy);
return () -> destinationRetriever.apply(strategy);
}

// deal with LOOKUP_THEN_EXCHANGE but exclude CURRENT_TENANT_THEN_PROVIDER for now
if( retrievalStrategy != CURRENT_TENANT_THEN_PROVIDER ) {
if( tokenExchangeStrategy == LOOKUP_THEN_EXCHANGE ) {
strategy = resolveSingleRequestStrategy(retrievalStrategy, LOOKUP_ONLY);
return () -> {
final ScpCfDestinationServiceV1Response result = destinationRetriever.apply(strategy);
Expand All @@ -206,62 +190,8 @@ Supplier<ScpCfDestinationServiceV1Response> prepareSupplier(
};
}

// handle CURRENT_TENANT_THEN_PROVIDER where current tenant != provider
return prepareSupplierForSubscriberThenProviderCase(tokenExchangeStrategy);
}

Supplier<ScpCfDestinationServiceV1Response> prepareSupplierForSubscriberThenProviderCase(
@Nonnull final ScpCfDestinationTokenExchangeStrategy tokenExchangeStrategy )
throws DestinationAccessException
{
// sanity check that this is never called with the provider tenant
if( currentTenantIsProvider() ) {
throw new IllegalStateException(
"Unexpected state: Preparing request to destination service for subscriber tenant, but current tenant is provider.");
}
final Strategy strategy;
final Strategy providerLookupOnlyStrategy = resolveSingleRequestStrategy(ALWAYS_PROVIDER, LOOKUP_ONLY);

if( tokenExchangeStrategy == LOOKUP_ONLY || tokenExchangeStrategy == FORWARD_USER_TOKEN ) {
strategy = resolveSingleRequestStrategy(ONLY_SUBSCRIBER, tokenExchangeStrategy);
return () -> {
try {
return destinationRetriever.apply(strategy);
}
catch( final DestinationNotFoundException e ) {
log
.debug(
"Did not find the destination in the subscriber account, falling back to the provider account.",
e);
return destinationRetriever.apply(providerLookupOnlyStrategy);
}
};
}

if( tokenExchangeStrategy == EXCHANGE_ONLY ) {
strategy = resolveSingleRequestStrategy(ONLY_SUBSCRIBER, EXCHANGE_ONLY);
return () -> destinationRetriever.apply(strategy);
}

// handle LOOKUP_THEN_EXCHANGE
strategy = resolveSingleRequestStrategy(ONLY_SUBSCRIBER, LOOKUP_ONLY);

return () -> {
try {
final ScpCfDestinationServiceV1Response result = destinationRetriever.apply(strategy);
if( !doesDestinationConfigurationRequireUserTokenExchange(result) ) {
return result;
}
return destinationRetriever.apply(tokenExchangeOnlyStrategy);
}
catch( final DestinationNotFoundException e ) {
log
.debug(
"Did not find the destination in the subscriber account, falling back to the provider account.",
e);
return destinationRetriever.apply(providerLookupOnlyStrategy);
}
};
strategy = resolveSingleRequestStrategy(retrievalStrategy, tokenExchangeStrategy);
return () -> destinationRetriever.apply(strategy);
}

private void warnOrThrowOnDeprecatedOrUnsupportedCombinations(
Expand Down Expand Up @@ -290,44 +220,6 @@ private void warnOrThrowOnDeprecatedOrUnsupportedCombinations(
LOOKUP_ONLY);
}
}
if( retrievalStrategy != CURRENT_TENANT_THEN_PROVIDER ) {
return;
}
log
.warn(
"The retrieval strategy {} is deprecated and should no longer be used."
+ " Please query subscriber and provider accounts individually using {} and {}",
CURRENT_TENANT_THEN_PROVIDER,
ONLY_SUBSCRIBER,
ALWAYS_PROVIDER);
if( currentTenantIsProvider() ) {
log
.warn(
"The retrieval strategy {} is used unnecessarily, the current tenant is the provider tenant."
+ " Only a single request will be made.",
CURRENT_TENANT_THEN_PROVIDER);
return;
}
if( tokenExchangeStrategy == null || tokenExchangeStrategy == LOOKUP_ONLY ) {
return;
}
log.warn("Option {} is not supported in conjunction with {}.", retrievalStrategy, tokenExchangeStrategy);
switch( tokenExchangeStrategy ) {
case EXCHANGE_ONLY: //fallthrough
log.warn("Falling back to {} with {}.", CURRENT_TENANT, EXCHANGE_ONLY);
break;
case FORWARD_USER_TOKEN:
case LOOKUP_THEN_EXCHANGE:
log
.warn(
"Attempting to apply {} for {}, hoping that destinations requiring a user token will only be present in the subscriber account."
+ " Potential requests to the provider account will not contain any user information.",
tokenExchangeStrategy,
CURRENT_TENANT_THEN_PROVIDER);
break;
default:
throw new IllegalStateException("Unexpected token strategy " + tokenExchangeStrategy);
}
}

Supplier<List<Destination>> prepareSupplierAllDestinations( @Nonnull final DestinationOptions options )
Expand All @@ -349,34 +241,20 @@ Supplier<List<Destination>> prepareSupplierAllDestinations( @Nonnull final Desti

Supplier<List<Destination>>
prepareSupplierAllDestinations( @Nonnull final ScpCfDestinationRetrievalStrategy strategy )
throws IllegalArgumentException
{
warnOrThrowOnDeprecatedOrUnsupportedCombinations(strategy, null);
switch( strategy ) {
case ALWAYS_PROVIDER: {
return () -> allDstinationRetriever.apply(TECHNICAL_USER_PROVIDER);
}
case CURRENT_TENANT_THEN_PROVIDER: {
return () -> {
try {
final List<Destination> destinations =
allDstinationRetriever.apply(TECHNICAL_USER_CURRENT_TENANT);
if( currentTenantIsProvider() || !destinations.isEmpty() ) {
return destinations;
}
}
catch( final Exception e ) {
log
.warn(
"Falling back to the provider tenant after failing to retrieve destinations for the subscriber tenant.");
log.debug("Lookup of all destinations for the subscriber tenant failed.", e);
}
return allDstinationRetriever.apply(TECHNICAL_USER_PROVIDER);
};
return () -> allDestinationRetriever.apply(TECHNICAL_USER_PROVIDER);
}
case ONLY_SUBSCRIBER:
case CURRENT_TENANT:
case CURRENT_TENANT: {
return () -> allDestinationRetriever.apply(TECHNICAL_USER_CURRENT_TENANT);
}
default: {
return () -> allDstinationRetriever.apply(TECHNICAL_USER_CURRENT_TENANT);
throw new IllegalArgumentException(
"The provided destination retrieval strategy " + strategy + " is not valid.");
}
}
}
Expand Down
13 changes: 0 additions & 13 deletions .../java/com/sap/cloud/sdk/cloudplatform/connectivity/ScpCfDestinationRetrievalStrategy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,6 @@
*/
public enum ScpCfDestinationRetrievalStrategy
{
/**
* Try to retrieve the current tenant's destination first. If the current tenant is a subscriber and no destination
* was found, fallback to the provider's destination. When loading all destinations both subaccount and instance
* level will be considered individually: If the current tenant is a subscriber and there are no destinations on the
* subaccount level, the subaccount level of the provider will be considered. Independently of that, if the current
* tenant is a subscriber and there are no destinations on the instance level, the provider instance level will be
* queried.
*
* @deprecated Please query subscriber and provider tenants individually instead using {@link #ONLY_SUBSCRIBER} and
* {@link #ALWAYS_PROVIDER}.
*/
@Deprecated
CURRENT_TENANT_THEN_PROVIDER("CurrentTenantThenProvider"),

/**
* Only load destination from the provider's sub-account, regardless if subscribers have a destination of the same
Expand Down
94 changes: 0 additions & 94 deletions ...om/sap/cloud/sdk/cloudplatform/connectivity/DestinationRetrievalStrategyResolverTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import static com.sap.cloud.sdk.cloudplatform.connectivity.OnBehalfOf.TECHNICAL_USER_PROVIDER;
import static com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationRetrievalStrategy.ALWAYS_PROVIDER;
import static com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationRetrievalStrategy.CURRENT_TENANT;
import static com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationRetrievalStrategy.CURRENT_TENANT_THEN_PROVIDER;
import static com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationRetrievalStrategy.ONLY_SUBSCRIBER;
import static com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationTokenExchangeStrategy.EXCHANGE_ONLY;
import static com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationTokenExchangeStrategy.FORWARD_USER_TOKEN;
Expand All @@ -20,7 +19,6 @@
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
Expand All @@ -41,7 +39,6 @@
import com.auth0.jwt.JWT;
import com.auth0.jwt.algorithms.Algorithm;
import com.sap.cloud.sdk.cloudplatform.connectivity.exception.DestinationAccessException;
import com.sap.cloud.sdk.cloudplatform.connectivity.exception.DestinationNotFoundException;
import com.sap.cloud.sdk.cloudplatform.security.AuthToken;
import com.sap.cloud.sdk.cloudplatform.security.AuthTokenAccessor;
import com.sap.cloud.sdk.cloudplatform.tenant.DefaultTenant;
Expand All @@ -52,7 +49,6 @@
import io.vavr.Tuple3;
import io.vavr.control.Try;

@SuppressWarnings( "deprecation" )
class DestinationRetrievalStrategyResolverTest
{
private static final Tenant providerT = new DefaultTenant("provider");
Expand Down Expand Up @@ -166,96 +162,6 @@ void testExceptionsAreThrownForImpossibleTokenExchanges()
verifyNoMoreInteractions(destinationRetriever);
}

@Test
@DisplayName( "When current tenant == provider then CURRENT_TENANT_THEN_PROVIDER should be equal to CURRENT_TENANT" )
void testCurrentThenProviderSimpleCase()
{
TenantAccessor
.executeWithTenant(
providerT,
() -> sut.prepareSupplier(CURRENT_TENANT_THEN_PROVIDER, LOOKUP_THEN_EXCHANGE));

verify(sut, times(1)).resolveSingleRequestStrategy(CURRENT_TENANT, LOOKUP_ONLY);

TenantAccessor
.executeWithTenant(
providerT,
() -> assertThatThrownBy(() -> sut.prepareSupplierForSubscriberThenProviderCase(LOOKUP_ONLY))
.isInstanceOf(IllegalStateException.class));
}

@Test
@DisplayName( "Test using CURRENT_TENANT_THEN_PROVIDER with LOOKUP_ONLY" )
void testSubThenProvLookupOnly()
{
doThrow(DestinationNotFoundException.class).when(destinationRetriever).apply(any());

// subscriber tenant is implied
assertThatThrownBy(sut.prepareSupplierForSubscriberThenProviderCase(LOOKUP_ONLY)::get)
.isInstanceOf(DestinationNotFoundException.class);

verify(destinationRetriever, times(1)).apply(eq(new Strategy(TECHNICAL_USER_CURRENT_TENANT, false)));
verify(destinationRetriever, times(1)).apply(eq(new Strategy(TECHNICAL_USER_PROVIDER, false)));
verifyNoMoreInteractions(destinationRetriever);
}

@Test
@DisplayName( "Test using CURRENT_TENANT_THEN_PROVIDER with FORWARD_USER_TOKEN" )
void testSubThenProvFwdUserToken()
{
doThrow(DestinationNotFoundException.class).when(destinationRetriever).apply(any());

// subscriber tenant is implied
assertThatThrownBy(sut.prepareSupplierForSubscriberThenProviderCase(FORWARD_USER_TOKEN)::get)
.isInstanceOf(DestinationNotFoundException.class);

verify(destinationRetriever, times(1)).apply(eq(new Strategy(TECHNICAL_USER_CURRENT_TENANT, true)));
verify(destinationRetriever, times(1)).apply(eq(new Strategy(TECHNICAL_USER_PROVIDER, false)));
verifyNoMoreInteractions(destinationRetriever);
}

@Test
@DisplayName( "Test using CURRENT_TENANT_THEN_PROVIDER with EXCHANGE_ONLY" )
void testSubThenProvExchangeOnly()
{
doAnswer(( any ) -> true).when(sut).doesDestinationConfigurationRequireUserTokenExchange(any());

// subscriber tenant is implied
sut.prepareSupplierForSubscriberThenProviderCase(EXCHANGE_ONLY).get();

verify(destinationRetriever, times(1)).apply(eq(new Strategy(NAMED_USER_CURRENT_TENANT, false)));
verifyNoMoreInteractions(destinationRetriever);
}

@Test
@DisplayName( "Test using CURRENT_TENANT_THEN_PROVIDER with LOOKUP_THEN_EXCHANGE" )
void testLookupThenExchangeWithCurrentTenantThenProvider()
{
doAnswer(( any ) -> true).when(sut).doesDestinationConfigurationRequireUserTokenExchange(any());

// subscriber tenant is implied
sut.prepareSupplierForSubscriberThenProviderCase(LOOKUP_THEN_EXCHANGE).get();

verify(destinationRetriever, times(1)).apply(eq(new Strategy(TECHNICAL_USER_CURRENT_TENANT, false)));
verify(destinationRetriever, times(1)).apply(eq(new Strategy(NAMED_USER_CURRENT_TENANT, false)));
verifyNoMoreInteractions(destinationRetriever);
}

@Test
@DisplayName( "Test getting all destinations with CURRENT_TENANT_THEN_PROVIDER" )
void testAllDestinationsCurrTenThenProv()
{
doThrow(DestinationAccessException.class).when(allDestinationRetriever).apply(TECHNICAL_USER_CURRENT_TENANT);

// subscriber tenant is implied
sut.prepareSupplierAllDestinations(CURRENT_TENANT_THEN_PROVIDER).get();

verify(allDestinationRetriever, times(1)).apply(TECHNICAL_USER_CURRENT_TENANT);
verify(allDestinationRetriever, times(1)).apply(TECHNICAL_USER_PROVIDER);

verifyNoMoreInteractions(allDestinationRetriever);
}

@Test
@DisplayName( "Test default strategies are set correctly" )
void testDefaultStrategies()
Expand Down
Loading

0 comments on commit d6c6664

Please sign in to comment.