Skip to content

Commit

Permalink
Fixed an issue where request-level signer overridden from execution i…
Browse files Browse the repository at this point in the history
…nterceptor is not honored (#5420)
  • Loading branch information
zoewangg committed Jul 24, 2024
1 parent 8e7d6f0 commit d206a72
Show file tree
Hide file tree
Showing 5 changed files with 172 additions and 70 deletions.
6 changes: 6 additions & 0 deletions .changes/next-release/bugfix-AWSSDKforJavav2-4303f87.json
Original file line number Diff line number Diff line change
@@ -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."
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -388,13 +423,21 @@ public void invokeInterceptorsAndCreateExecutionContext_requestOverrideForIdenti
}

private ClientExecutionParams<SdkRequest, SdkResponse> clientExecutionParams() {
return clientExecutionParams(sdkRequest);
}

private ClientExecutionParams<SdkRequest, SdkResponse> clientExecutionParams(SdkRequest sdkRequest) {
return new ClientExecutionParams<SdkRequest, SdkResponse>()
.withInput(sdkRequest)
.withFullDuplex(false)
.withOperationName("TestOperation");
}

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.
Expand All @@ -408,9 +451,8 @@ private SdkClientConfiguration.Builder testClientConfiguration() {
.put(SdkInternalExecutionAttribute.SELECTED_AUTH_SCHEME, selectedAuthScheme)
.build();

List<ExecutionInterceptor> 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);
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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).

}

0 comments on commit d206a72

Please sign in to comment.