From 7733ccc0886d49b70184930728e3753e73586b6e Mon Sep 17 00:00:00 2001 From: Jaykumar Gosar Date: Sun, 24 Sep 2023 02:26:08 -0700 Subject: [PATCH 1/3] Fix isClientSignerOverridden to check actual value --- .../amazon/awssdk/awscore/util/SignerOverrideUtils.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/util/SignerOverrideUtils.java b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/util/SignerOverrideUtils.java index 6526d05ccdb1..58b84d015a73 100644 --- a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/util/SignerOverrideUtils.java +++ b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/util/SignerOverrideUtils.java @@ -66,12 +66,14 @@ public static SdkRequest overrideSignerIfNotOverridden(SdkRequest request, * @deprecated No longer used by modern clients after migration to reference architecture */ @Deprecated + // TODO(sra-identity-and-auth): These used to be used by EndpointsAuthSchemeInterceptor, which has now been removed, but + // this method is still used from AwsExecutionContextBuilder. Should @Deprecated be removed? public static boolean isSignerOverridden(SdkRequest request, ExecutionAttributes executionAttributes) { - Optional isClientSignerOverridden = Optional.ofNullable( - executionAttributes.getAttribute(SdkExecutionAttribute.SIGNER_OVERRIDDEN)); + boolean isClientSignerOverridden = + Boolean.TRUE.equals(executionAttributes.getAttribute(SdkExecutionAttribute.SIGNER_OVERRIDDEN)); Optional requestSigner = request.overrideConfiguration() .flatMap(RequestOverrideConfiguration::signer); - return isClientSignerOverridden.isPresent() || requestSigner.isPresent(); + return isClientSignerOverridden || requestSigner.isPresent(); } /** From 0527e2204d4d475945fd36ab2787a4d02be45467 Mon Sep 17 00:00:00 2001 From: Jaykumar Gosar Date: Sun, 24 Sep 2023 16:00:20 -0700 Subject: [PATCH 2/3] Add unit test that shows the issue before fix --- .../services/SraIdentityResolutionTest.java | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/SraIdentityResolutionTest.java diff --git a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/SraIdentityResolutionTest.java b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/SraIdentityResolutionTest.java new file mode 100644 index 000000000000..0929774fcd4b --- /dev/null +++ b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/SraIdentityResolutionTest.java @@ -0,0 +1,67 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.awssdk.services; + +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +import java.util.concurrent.CompletableFuture; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; +import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; +import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; +import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration; +import software.amazon.awssdk.identity.spi.AwsCredentialsIdentity; +import software.amazon.awssdk.identity.spi.ResolveIdentityRequest; +import software.amazon.awssdk.services.protocolrestjson.ProtocolRestJsonClient; + +@RunWith(MockitoJUnitRunner.class) +public class SraIdentityResolutionTest { + + @Mock + private AwsCredentialsProvider credsProvider; + + @Test + public void testIdentityPropertyBasedResolutionIsUsedAndNotAnotherIdentityResolution() { + when(credsProvider.identityType()).thenReturn(AwsCredentialsIdentity.class); + when(credsProvider.resolveIdentity(any(ResolveIdentityRequest.class))) + .thenReturn(CompletableFuture.completedFuture(AwsBasicCredentials.create("akid1", "skid2"))); + ProtocolRestJsonClient syncClient = ProtocolRestJsonClient + .builder() + .credentialsProvider(credsProvider) + // Below is necessary to create the test case where, addCredentialsToExecutionAttributes was getting called before + .overrideConfiguration(ClientOverrideConfiguration.builder().build()) + .build(); + + try { + syncClient.allTypes(); + } catch (Exception expected) { + } + + verify(credsProvider, times(2)).identityType(); + + // This asserts that the identity used is the one from resolveIdentity() called by SRA AuthSchemeInterceptor and not from + // from another call like from AwsCredentialsAuthorizationStrategy.addCredentialsToExecutionAttributes, asserted by + // combination of times(1) and verifyNoMoreInteractions. + verify(credsProvider, times(1)).resolveIdentity(any(ResolveIdentityRequest.class)); + verifyNoMoreInteractions(credsProvider); + } +} From c04936a0246dabd868af7f95363dbd2cc84d4066 Mon Sep 17 00:00:00 2001 From: Jaykumar Gosar Date: Tue, 26 Sep 2023 13:50:16 -0700 Subject: [PATCH 3/3] Undeprecated used SignerOverrideUtils method --- .../amazon/awssdk/awscore/util/SignerOverrideUtils.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/util/SignerOverrideUtils.java b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/util/SignerOverrideUtils.java index 58b84d015a73..8ec31e32bd15 100644 --- a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/util/SignerOverrideUtils.java +++ b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/util/SignerOverrideUtils.java @@ -30,11 +30,8 @@ /** * Utility to override a given {@link SdkRequest}'s {@link Signer}. Typically used by {@link ExecutionInterceptor}s that wish to * dynamically enable particular signing methods, like SigV4a for multi-region endpoints. - * - * @deprecated No longer used by modern clients after migration to reference architecture */ @SdkProtectedApi -@Deprecated public final class SignerOverrideUtils { private SignerOverrideUtils() { } @@ -62,12 +59,6 @@ public static SdkRequest overrideSignerIfNotOverridden(SdkRequest request, return overrideSigner(request, signer.get()); } - /** - * @deprecated No longer used by modern clients after migration to reference architecture - */ - @Deprecated - // TODO(sra-identity-and-auth): These used to be used by EndpointsAuthSchemeInterceptor, which has now been removed, but - // this method is still used from AwsExecutionContextBuilder. Should @Deprecated be removed? public static boolean isSignerOverridden(SdkRequest request, ExecutionAttributes executionAttributes) { boolean isClientSignerOverridden = Boolean.TRUE.equals(executionAttributes.getAttribute(SdkExecutionAttribute.SIGNER_OVERRIDDEN));