From d206a7269758b67185c0773d257583c32d370671 Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Tue, 23 Jul 2024 17:14:55 -0700 Subject: [PATCH] Fixed an issue where request-level signer overridden from execution interceptor is not honored (#5420) --- .../bugfix-AWSSDKforJavav2-4303f87.json | 6 + .../internal/AwsExecutionContextBuilder.java | 5 +- .../AwsExecutionContextBuilderTest.java | 46 ++++++- .../services/AsyncSignerOverrideTest.java | 66 ---------- .../awssdk/services/SignerOverrideTest.java | 119 ++++++++++++++++++ 5 files changed, 172 insertions(+), 70 deletions(-) create mode 100644 .changes/next-release/bugfix-AWSSDKforJavav2-4303f87.json delete mode 100644 test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/AsyncSignerOverrideTest.java create mode 100644 test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/SignerOverrideTest.java diff --git a/.changes/next-release/bugfix-AWSSDKforJavav2-4303f87.json b/.changes/next-release/bugfix-AWSSDKforJavav2-4303f87.json new file mode 100644 index 000000000000..5319ac65eabf --- /dev/null +++ b/.changes/next-release/bugfix-AWSSDKforJavav2-4303f87.json @@ -0,0 +1,6 @@ +{ + "type": "bugfix", + "category": "AWS SDK for Java v2", + "contributor": "", + "description": "Fixed the issue where request-level signer override provided through ExecutionInterceptor is not honored." +} diff --git a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/internal/AwsExecutionContextBuilder.java b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/internal/AwsExecutionContextBuilder.java index 858120adae54..e3e273aa03d7 100644 --- a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/internal/AwsExecutionContextBuilder.java +++ b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/internal/AwsExecutionContextBuilder.java @@ -131,10 +131,11 @@ private AwsExecutionContextBuilder() { .build(); interceptorContext = runInitialInterceptors(interceptorContext, executionAttributes, executionInterceptorChain); + SdkRequest modifiedRequests = interceptorContext.request(); Signer signer = null; - if (loadOldSigner(executionAttributes, originalRequest)) { + if (loadOldSigner(executionAttributes, modifiedRequests)) { AuthorizationStrategyFactory authorizationStrategyFactory = - new AuthorizationStrategyFactory(interceptorContext.request(), metricCollector, clientConfig); + new AuthorizationStrategyFactory(modifiedRequests, metricCollector, clientConfig); AuthorizationStrategy authorizationStrategy = authorizationStrategyFactory.strategyFor(executionParams.credentialType()); authorizationStrategy.addCredentialsToExecutionAttributes(executionAttributes); diff --git a/core/aws-core/src/test/java/software/amazon/awssdk/awscore/internal/AwsExecutionContextBuilderTest.java b/core/aws-core/src/test/java/software/amazon/awssdk/awscore/internal/AwsExecutionContextBuilderTest.java index 91a18bfc8477..4f2bea548c95 100644 --- a/core/aws-core/src/test/java/software/amazon/awssdk/awscore/internal/AwsExecutionContextBuilderTest.java +++ b/core/aws-core/src/test/java/software/amazon/awssdk/awscore/internal/AwsExecutionContextBuilderTest.java @@ -22,6 +22,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map; @@ -35,8 +36,10 @@ import org.mockito.junit.MockitoJUnitRunner; import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; +import software.amazon.awssdk.awscore.AwsRequest; import software.amazon.awssdk.awscore.AwsRequestOverrideConfiguration; import software.amazon.awssdk.awscore.client.config.AwsClientOption; +import software.amazon.awssdk.awscore.client.http.NoopTestAwsRequest; import software.amazon.awssdk.core.SdkRequest; import software.amazon.awssdk.core.SdkResponse; import software.amazon.awssdk.core.SelectedAuthScheme; @@ -46,22 +49,26 @@ import software.amazon.awssdk.core.client.config.SdkClientOption; import software.amazon.awssdk.core.client.handler.ClientExecutionParams; import software.amazon.awssdk.core.http.ExecutionContext; +import software.amazon.awssdk.core.interceptor.Context; import software.amazon.awssdk.core.interceptor.ExecutionAttributes; import software.amazon.awssdk.core.interceptor.ExecutionInterceptor; import software.amazon.awssdk.core.interceptor.SdkExecutionAttribute; import software.amazon.awssdk.core.interceptor.SdkInternalExecutionAttribute; import software.amazon.awssdk.core.interceptor.trait.HttpChecksum; import software.amazon.awssdk.core.internal.util.HttpChecksumUtils; +import software.amazon.awssdk.core.signer.NoOpSigner; import software.amazon.awssdk.core.signer.Signer; import software.amazon.awssdk.http.auth.aws.scheme.AwsV4AuthScheme; import software.amazon.awssdk.http.auth.scheme.NoAuthAuthScheme; import software.amazon.awssdk.http.auth.spi.scheme.AuthScheme; import software.amazon.awssdk.http.auth.spi.scheme.AuthSchemeOption; import software.amazon.awssdk.http.auth.spi.signer.HttpSigner; +import software.amazon.awssdk.http.auth.spi.signer.SignerProperty; import software.amazon.awssdk.identity.spi.AwsCredentialsIdentity; import software.amazon.awssdk.identity.spi.IdentityProvider; import software.amazon.awssdk.identity.spi.IdentityProviders; import software.amazon.awssdk.profiles.ProfileFile; +import software.amazon.awssdk.regions.RegionScope; @RunWith(MockitoJUnitRunner.class) public class AwsExecutionContextBuilderTest { @@ -177,6 +184,34 @@ public void postSra_signing_ifClientOverride_assignClientOverrideSigner_resolveI verify(defaultCredentialsProvider, times(1)).resolveIdentity(); } + @Test + public void postSra_oldSignerOverriddenThroughExecutionInterceptor_shouldTakePrecedence() { + SdkRequest request = NoopTestAwsRequest.builder().build(); + + Signer noOpSigner = new NoOpSigner(); + ExecutionInterceptor signerExecutionInterceptor = signerOverrideExecutionInterceptor(noOpSigner); + SdkClientConfiguration configuration = testClientConfiguration(signerExecutionInterceptor).build(); + ExecutionContext executionContext = + AwsExecutionContextBuilder.invokeInterceptorsAndCreateExecutionContext(clientExecutionParams(request), + configuration); + + assertThat(executionContext.signer()).isEqualTo(noOpSigner); + verify(defaultCredentialsProvider, times(1)).resolveIdentity(); + } + + private ExecutionInterceptor signerOverrideExecutionInterceptor(Signer signer) { + return new ExecutionInterceptor() { + @Override + public SdkRequest modifyRequest(Context.ModifyRequest context, ExecutionAttributes executionAttributes) { + AwsRequest.Builder builder = (AwsRequest.Builder) context.request().toBuilder(); + builder.overrideConfiguration(c -> c.signer(signer) + .build()); + + return builder.build(); + } + }; + } + @Test public void preSra_authTypeNone_doesNotAssignSigner_doesNotResolveIdentity() { SdkClientConfiguration.Builder clientConfig = preSraClientConfiguration(); @@ -388,6 +423,10 @@ public void invokeInterceptorsAndCreateExecutionContext_requestOverrideForIdenti } private ClientExecutionParams clientExecutionParams() { + return clientExecutionParams(sdkRequest); + } + + private ClientExecutionParams clientExecutionParams(SdkRequest sdkRequest) { return new ClientExecutionParams() .withInput(sdkRequest) .withFullDuplex(false) @@ -395,6 +434,10 @@ private ClientExecutionParams clientExecutionParams() { } private SdkClientConfiguration.Builder testClientConfiguration() { + return testClientConfiguration(interceptor); + } + + private SdkClientConfiguration.Builder testClientConfiguration(ExecutionInterceptor... executionInterceptors) { // In real SRA case, SelectedAuthScheme is setup as an executionAttribute by {Service}AuthSchemeInterceptor that is setup // in EXECUTION_INTERCEPTORS. But, faking it here for unit test, by already setting SELECTED_AUTH_SCHEME into the // executionAttributes. @@ -408,9 +451,8 @@ private SdkClientConfiguration.Builder testClientConfiguration() { .put(SdkInternalExecutionAttribute.SELECTED_AUTH_SCHEME, selectedAuthScheme) .build(); - List interceptorList = Collections.singletonList(interceptor); return SdkClientConfiguration.builder() - .option(SdkClientOption.EXECUTION_INTERCEPTORS, interceptorList) + .option(SdkClientOption.EXECUTION_INTERCEPTORS, Arrays.asList(executionInterceptors)) .option(AwsClientOption.CREDENTIALS_IDENTITY_PROVIDER, defaultCredentialsProvider) .option(SdkClientOption.AUTH_SCHEMES, defaultAuthSchemes) .option(SdkClientOption.EXECUTION_ATTRIBUTES, executionAttributes); diff --git a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/AsyncSignerOverrideTest.java b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/AsyncSignerOverrideTest.java deleted file mode 100644 index f1d6b9257d02..000000000000 --- a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/AsyncSignerOverrideTest.java +++ /dev/null @@ -1,66 +0,0 @@ -/* - * 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.ArgumentMatchers.any; -import static org.mockito.Mockito.verify; -import static software.amazon.awssdk.core.client.config.SdkAdvancedClientOption.SIGNER; - -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.StaticCredentialsProvider; -import software.amazon.awssdk.core.async.AsyncRequestBody; -import software.amazon.awssdk.core.interceptor.ExecutionAttributes; -import software.amazon.awssdk.core.signer.Signer; -import software.amazon.awssdk.http.SdkHttpFullRequest; -import software.amazon.awssdk.regions.Region; -import software.amazon.awssdk.services.protocolrestjson.ProtocolRestJsonAsyncClient; -import software.amazon.awssdk.services.protocolrestjson.model.StreamingInputOperationRequest; - -/** - * Test to ensure that operations that use the {@link software.amazon.awssdk.auth.signer.AsyncAws4Signer} don't apply - * the override when the signer is overridden by the customer. - */ -@RunWith(MockitoJUnitRunner.class) -public class AsyncSignerOverrideTest { - @Mock - public Signer mockSigner; - - @Test - public void test_signerOverriddenForStreamingInput_takesPrecedence() { - ProtocolRestJsonAsyncClient asyncClient = ProtocolRestJsonAsyncClient.builder() - .credentialsProvider(StaticCredentialsProvider.create(AwsBasicCredentials.create("akid", "skid"))) - .region(Region.US_WEST_2) - .overrideConfiguration(o -> o.putAdvancedOption(SIGNER, mockSigner)) - .build(); - - try { - asyncClient.streamingInputOperation(StreamingInputOperationRequest.builder().build(), - AsyncRequestBody.fromString("test")).join(); - } catch (Exception expected) { - } - - verify(mockSigner).sign(any(SdkHttpFullRequest.class), any(ExecutionAttributes.class)); - } - - // TODO(sra-identity-and-auth): Add test for SRA way of overriding signer to assert that overridden signer is used. - // At that point, rename this class to SignerOverrideTest, not specific to AsyncSignerOverride (which was for operation - // level codegen changes). - -} diff --git a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/SignerOverrideTest.java b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/SignerOverrideTest.java new file mode 100644 index 000000000000..5b30c9400828 --- /dev/null +++ b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/SignerOverrideTest.java @@ -0,0 +1,119 @@ +/* + * 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.ArgumentMatchers.any; +import static org.mockito.Mockito.verify; +import static software.amazon.awssdk.core.client.config.SdkAdvancedClientOption.SIGNER; + +import java.net.URI; +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.StaticCredentialsProvider; +import software.amazon.awssdk.awscore.AwsRequest; +import software.amazon.awssdk.core.SdkRequest; +import software.amazon.awssdk.core.async.AsyncRequestBody; +import software.amazon.awssdk.core.interceptor.Context; +import software.amazon.awssdk.core.interceptor.ExecutionAttributes; +import software.amazon.awssdk.core.interceptor.ExecutionInterceptor; +import software.amazon.awssdk.core.signer.Signer; +import software.amazon.awssdk.http.SdkHttpFullRequest; +import software.amazon.awssdk.regions.Region; +import software.amazon.awssdk.services.protocolrestjson.ProtocolRestJsonAsyncClient; +import software.amazon.awssdk.services.protocolrestjson.ProtocolRestJsonClient; +import software.amazon.awssdk.services.protocolrestjson.model.AllTypesRequest; +import software.amazon.awssdk.services.protocolrestjson.model.StreamingInputOperationRequest; + + +@RunWith(MockitoJUnitRunner.class) +public class SignerOverrideTest { + @Mock + public Signer mockSigner; + + /** + * Test to ensure that operations that use the {@link software.amazon.awssdk.auth.signer.AsyncAws4Signer} don't apply + * the override when the signer is overridden by the customer. + */ + @Test + public void test_signerOverriddenForStreamingInput_takesPrecedence() { + ProtocolRestJsonAsyncClient asyncClient = ProtocolRestJsonAsyncClient.builder() + .credentialsProvider(StaticCredentialsProvider.create(AwsBasicCredentials.create("akid", "skid"))) + .region(Region.US_WEST_2) + .overrideConfiguration(o -> o.putAdvancedOption(SIGNER, mockSigner)) + .build(); + + try { + asyncClient.streamingInputOperation(StreamingInputOperationRequest.builder().build(), + AsyncRequestBody.fromString("test")).join(); + } catch (Exception expected) { + } + + verify(mockSigner).sign(any(SdkHttpFullRequest.class), any(ExecutionAttributes.class)); + } + + @Test + public void asyncClient_oldSignerOverriddenInExecutionInterceptor_takesPrecedence() { + try (ProtocolRestJsonAsyncClient asyncClient = ProtocolRestJsonAsyncClient.builder() + .credentialsProvider(StaticCredentialsProvider.create(AwsBasicCredentials.create("akid", "skid"))) + .region(Region.US_WEST_2) + .endpointOverride(URI.create("http://localhost:8080")) + .overrideConfiguration(o -> o.addExecutionInterceptor(signerOverrideExecutionInterceptor(mockSigner))) + .build()) { + asyncClient.allTypes(AllTypesRequest.builder().build()).join(); + } catch (Exception expected) { + // Doesn't matter if the request succeeds or not + } + + verify(mockSigner).sign(any(SdkHttpFullRequest.class), any(ExecutionAttributes.class)); + } + + @Test + public void syncClient_oldSignerOverriddenInExecutionInterceptor_takesPrecedence() { + try (ProtocolRestJsonClient client = ProtocolRestJsonClient.builder() + .credentialsProvider(StaticCredentialsProvider.create(AwsBasicCredentials.create("akid", "skid"))) + .region(Region.US_WEST_2) + .endpointOverride(URI.create("http://localhost:8080")) + .overrideConfiguration(o -> o.addExecutionInterceptor(signerOverrideExecutionInterceptor(mockSigner))) + .build()) { + client.allTypes(AllTypesRequest.builder().build()); + } catch (Exception expected) { + // Doesn't matter if the request succeeds or not + } + + verify(mockSigner).sign(any(SdkHttpFullRequest.class), any(ExecutionAttributes.class)); + } + + private ExecutionInterceptor signerOverrideExecutionInterceptor(Signer signer) { + return new ExecutionInterceptor() { + @Override + public SdkRequest modifyRequest(Context.ModifyRequest context, ExecutionAttributes executionAttributes) { + AwsRequest.Builder builder = (AwsRequest.Builder) context.request().toBuilder(); + builder.overrideConfiguration(c -> c.signer(signer) + .build()); + + return builder.build(); + } + }; + } + + // TODO(sra-identity-and-auth): Add test for SRA way of overriding signer to assert that overridden signer is used. + // At that point, rename this class to SignerOverrideTest, not specific to AsyncSignerOverride (which was for operation + // level codegen changes). + +}