diff --git a/core/auth/src/main/java/software/amazon/awssdk/auth/signer/S3SignerExecutionAttribute.java b/core/auth/src/main/java/software/amazon/awssdk/auth/signer/S3SignerExecutionAttribute.java index 33284eaecab6..e35c1f8d4f89 100644 --- a/core/auth/src/main/java/software/amazon/awssdk/auth/signer/S3SignerExecutionAttribute.java +++ b/core/auth/src/main/java/software/amazon/awssdk/auth/signer/S3SignerExecutionAttribute.java @@ -90,8 +90,8 @@ private static SelectedAuthScheme enableChunkedEncodingW Boolean enableChunkedEncoding) { if (authScheme == null) { // This is an unusual use-case. - // Let's assume they're setting normalize-path so that they can call the signer directly. If that's true, then it - // doesn't really matter what we store other than normalize-path. + // Let's assume they're setting chunked-encoding so that they can call the signer directly. If that's true, then it + // doesn't really matter what we store other than chunked-encoding. return new SelectedAuthScheme<>(CompletableFuture.completedFuture(new UnsetIdentity()), new UnsetHttpSigner(), AuthSchemeOption.builder() diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/SigningStage.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/SigningStage.java index bd4c9285f837..e755a3c07ab7 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/SigningStage.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/SigningStage.java @@ -29,7 +29,6 @@ import software.amazon.awssdk.core.internal.http.InterruptMonitor; import software.amazon.awssdk.core.internal.http.RequestExecutionContext; import software.amazon.awssdk.core.internal.http.pipeline.RequestToRequestPipeline; -import software.amazon.awssdk.core.internal.http.pipeline.stages.utils.SignerOverrideUtils; import software.amazon.awssdk.core.internal.util.MetricUtils; import software.amazon.awssdk.core.metrics.CoreMetric; import software.amazon.awssdk.core.signer.AsyncRequestBodySigner; @@ -64,13 +63,11 @@ public SigningStage(HttpClientDependencies dependencies) { @Override public SdkHttpFullRequest execute(SdkHttpFullRequest request, RequestExecutionContext context) throws Exception { InterruptMonitor.checkInterrupted(); - if (shouldDoSraSigning(context)) { - SelectedAuthScheme selectedAuthScheme = context - .executionAttributes() - .getAttribute(SdkInternalExecutionAttribute.SELECTED_AUTH_SCHEME); - return sraSignRequest(request, - context, - selectedAuthScheme); + + SelectedAuthScheme selectedAuthScheme = + context.executionAttributes().getAttribute(SdkInternalExecutionAttribute.SELECTED_AUTH_SCHEME); + if (shouldDoSraSigning(context, selectedAuthScheme)) { + return sraSignRequest(request, context, selectedAuthScheme); } return signRequest(request, context); } @@ -169,9 +166,8 @@ private void updateHttpRequestInInterceptorContext(SdkHttpFullRequest request, E } /** - * We sign if a signer is provided is not null. - * - * @return True if request should be signed, false if not. + * We sign if it isn't auth=none. This attribute is no longer set in the SRA, so this exists only for old clients. In + * addition to this, old clients only set this to false, never true. So, we have to treat null as true. */ private boolean shouldSign(ExecutionAttributes attributes, Signer signer) { return signer != null && @@ -180,13 +176,9 @@ private boolean shouldSign(ExecutionAttributes attributes, Signer signer) { /** * Returns true if we should use SRA signing logic. - * If there is a SELECTED_AUTH_SCHEME, it implies it's a newly (with SRA) generated clients. - * If Signer is overridden, with either old or new clients, it still means it's using pre SRA interfaces, so falls back to - * pre SRA signing logic. */ - private boolean shouldDoSraSigning(RequestExecutionContext context) { - return !SignerOverrideUtils.isSignerOverridden(context) - && context.executionAttributes().getAttribute(SdkInternalExecutionAttribute.SELECTED_AUTH_SCHEME) != null; + private boolean shouldDoSraSigning(RequestExecutionContext context, SelectedAuthScheme selectedAuthScheme) { + return context.signer() == null && selectedAuthScheme != null; } /** diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/utils/SignerOverrideUtils.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/utils/SignerOverrideUtils.java deleted file mode 100644 index 3daa032e8405..000000000000 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/utils/SignerOverrideUtils.java +++ /dev/null @@ -1,48 +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.core.internal.http.pipeline.stages.utils; - -import java.util.Optional; -import software.amazon.awssdk.annotations.SdkInternalApi; -import software.amazon.awssdk.core.RequestOverrideConfiguration; -import software.amazon.awssdk.core.interceptor.SdkExecutionAttribute; -import software.amazon.awssdk.core.internal.http.RequestExecutionContext; -import software.amazon.awssdk.core.internal.http.pipeline.stages.AsyncSigningStage; -import software.amazon.awssdk.core.internal.http.pipeline.stages.SigningStage; -import software.amazon.awssdk.core.signer.Signer; - -/** - * Utility to share across {@link SigningStage} and {@link AsyncSigningStage}. - */ -@SdkInternalApi -public final class SignerOverrideUtils { - - private SignerOverrideUtils() { - } - - // This is (mostly) same as software.amazon.awssdk.awscore.util.SignerOverrideUtils.isSignerOverridden, but since that - // is in aws-core, and this is in sdk-core, copied here. It is "mostly" because it changes the SIGNER_OVERRIDDEN check to - // `true` instead of just isPresent(). - public static boolean isSignerOverridden(RequestExecutionContext context) { - boolean isClientSignerOverridden = - Boolean.TRUE.equals(context.executionAttributes().getAttribute(SdkExecutionAttribute.SIGNER_OVERRIDDEN)); - - Optional requestSigner = context.originalRequest() - .overrideConfiguration() - .flatMap(RequestOverrideConfiguration::signer); - return isClientSignerOverridden || requestSigner.isPresent(); - } -} diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/http/pipeline/stages/AsyncSigningStageTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/http/pipeline/stages/AsyncSigningStageTest.java index 1adfef0c191e..6196fc2ccec4 100644 --- a/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/http/pipeline/stages/AsyncSigningStageTest.java +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/http/pipeline/stages/AsyncSigningStageTest.java @@ -23,7 +23,6 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; -import static software.amazon.awssdk.core.interceptor.SdkExecutionAttribute.SIGNER_OVERRIDDEN; import static software.amazon.awssdk.core.interceptor.SdkExecutionAttribute.TIME_OFFSET; import static software.amazon.awssdk.core.interceptor.SdkInternalExecutionAttribute.SELECTED_AUTH_SCHEME; import static software.amazon.awssdk.core.metrics.CoreMetric.SIGNING_DURATION; @@ -37,18 +36,16 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; +import org.mockito.ArgumentMatchers; import org.mockito.Captor; import org.mockito.Mock; -import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; import org.reactivestreams.Publisher; import software.amazon.awssdk.core.SdkRequest; -import software.amazon.awssdk.core.SdkRequestOverrideConfiguration; import software.amazon.awssdk.core.SelectedAuthScheme; import software.amazon.awssdk.core.async.AsyncRequestBody; import software.amazon.awssdk.core.client.config.SdkClientConfiguration; import software.amazon.awssdk.core.http.ExecutionContext; -import software.amazon.awssdk.core.http.NoopTestRequest; import software.amazon.awssdk.core.interceptor.ExecutionAttributes; import software.amazon.awssdk.core.interceptor.InterceptorContext; import software.amazon.awssdk.core.internal.http.HttpClientDependencies; @@ -58,13 +55,13 @@ import software.amazon.awssdk.core.signer.Signer; import software.amazon.awssdk.http.SdkHttpFullRequest; import software.amazon.awssdk.http.SdkHttpRequest; +import software.amazon.awssdk.http.auth.spi.scheme.AuthSchemeOption; import software.amazon.awssdk.http.auth.spi.signer.AsyncSignRequest; import software.amazon.awssdk.http.auth.spi.signer.AsyncSignedRequest; -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.http.auth.spi.signer.SignRequest; import software.amazon.awssdk.http.auth.spi.signer.SignedRequest; +import software.amazon.awssdk.http.auth.spi.signer.SignerProperty; import software.amazon.awssdk.identity.spi.Identity; import software.amazon.awssdk.metrics.MetricCollector; import utils.ValidSdkObjects; @@ -78,7 +75,7 @@ public class AsyncSigningStageTest { private Identity identity; @Mock - private HttpSigner signer; + private HttpSigner httpSigner; @Mock private Signer oldSigner; @@ -107,19 +104,19 @@ public void setup() { } @Test - public void execute_selectedAuthScheme_doesSraSign() throws Exception { + public void execute_selectedAuthScheme_nullSigner_doesSraSign() throws Exception { // Set up a scheme with a signer property SelectedAuthScheme selectedAuthScheme = new SelectedAuthScheme<>( CompletableFuture.completedFuture(identity), - signer, + httpSigner, AuthSchemeOption.builder() .schemeId("my.auth#myAuth") .putSignerProperty(SIGNER_PROPERTY, "value") .build()); - RequestExecutionContext context = createContext(selectedAuthScheme, (Signer) null); + RequestExecutionContext context = createContext(selectedAuthScheme, null); SdkHttpRequest signedRequest = ValidSdkObjects.sdkHttpFullRequest().build(); - when(signer.sign(Mockito.>any())) + when(httpSigner.sign(ArgumentMatchers.>any())) .thenReturn(SignedRequest.builder() .request(signedRequest) .build()); @@ -132,7 +129,7 @@ public void execute_selectedAuthScheme_doesSraSign() throws Exception { assertThat(context.executionContext().interceptorContext().httpRequest()).isSameAs(result); // assert that the input to the signer is as expected, including that signer properties are set - verify(signer).sign(signRequestCaptor.capture()); + verify(httpSigner).sign(signRequestCaptor.capture()); SignRequest signRequest = signRequestCaptor.getValue(); assertThat(signRequest.identity()).isSameAs(identity); assertThat(signRequest.request()).isSameAs(request); @@ -150,12 +147,12 @@ public void execute_selectedAuthScheme_doesSraSign() throws Exception { } @Test - public void execute_selectedAuthSchemeAndTimeOffsetSet_doesSraSignAndAdjustTheSigningClock() throws Exception { + public void execute_selectedAuthScheme_nullSigner_timeOffsetSet_doesSraSignAndAdjustTheSigningClock() throws Exception { AsyncRequestBody asyncPayload = AsyncRequestBody.fromString("async request body"); // Set up a scheme with a signer property SelectedAuthScheme selectedAuthScheme = new SelectedAuthScheme<>( CompletableFuture.completedFuture(identity), - signer, + httpSigner, AuthSchemeOption.builder() .schemeId("my.auth#myAuth") .putSignerProperty(SIGNER_PROPERTY, "value") @@ -164,7 +161,7 @@ public void execute_selectedAuthSchemeAndTimeOffsetSet_doesSraSignAndAdjustTheSi SdkHttpRequest signedRequest = ValidSdkObjects.sdkHttpFullRequest().build(); Publisher signedPayload = AsyncRequestBody.fromString("signed async request body"); - when(signer.signAsync(Mockito.>any())) + when(httpSigner.signAsync(ArgumentMatchers.>any())) .thenReturn( CompletableFuture.completedFuture(AsyncSignedRequest.builder() .request(signedRequest) @@ -181,7 +178,7 @@ public void execute_selectedAuthSchemeAndTimeOffsetSet_doesSraSignAndAdjustTheSi assertThat(context.executionContext().interceptorContext().httpRequest()).isSameAs(result); // assert that the input to the signer is as expected, including that signer properties are set - verify(signer).signAsync(asyncSignRequestCaptor.capture()); + verify(httpSigner).signAsync(asyncSignRequestCaptor.capture()); AsyncSignRequest signRequest = asyncSignRequestCaptor.getValue(); assertThat(signRequest.identity()).isSameAs(identity); assertThat(signRequest.request()).isSameAs(request); @@ -200,13 +197,13 @@ public void execute_selectedAuthSchemeAndTimeOffsetSet_doesSraSignAndAdjustTheSi } @Test - public void execute_selectedAuthScheme_doesSraSignAndDoesNotOverrideAuthSchemeOptionClock() throws Exception { + public void execute_selectedAuthScheme_nullSigner_doesSraSignAndDoesNotOverrideAuthSchemeOptionClock() throws Exception { AsyncRequestBody asyncPayload = AsyncRequestBody.fromString("async request body"); // Set up a scheme with a signer property and the signing clock set Clock clock = SigningStageTest.testClock(); SelectedAuthScheme selectedAuthScheme = new SelectedAuthScheme<>( CompletableFuture.completedFuture(identity), - signer, + httpSigner, AuthSchemeOption.builder() .schemeId("my.auth#myAuth") .putSignerProperty(SIGNER_PROPERTY, "value") @@ -217,7 +214,7 @@ public void execute_selectedAuthScheme_doesSraSignAndDoesNotOverrideAuthSchemeOp RequestExecutionContext context = createContext(selectedAuthScheme, asyncPayload, null); SdkHttpRequest signedRequest = ValidSdkObjects.sdkHttpFullRequest().build(); - when(signer.signAsync(Mockito.>any())) + when(httpSigner.signAsync(ArgumentMatchers.>any())) .thenReturn( CompletableFuture.completedFuture(AsyncSignedRequest.builder() .request(signedRequest) @@ -231,7 +228,7 @@ public void execute_selectedAuthScheme_doesSraSignAndDoesNotOverrideAuthSchemeOp assertThat(context.executionContext().interceptorContext().httpRequest()).isSameAs(result); // assert that the input to the signer is as expected, including that signer properties are set - verify(signer).signAsync(asyncSignRequestCaptor.capture()); + verify(httpSigner).signAsync(asyncSignRequestCaptor.capture()); AsyncSignRequest signRequest = asyncSignRequestCaptor.getValue(); assertThat(signRequest.identity()).isSameAs(identity); assertThat(signRequest.request()).isSameAs(request); @@ -258,7 +255,7 @@ public void execute_selectedAuthScheme_asyncRequestBody_doesSraSignAsync() throw // Set up a scheme with a signer property SelectedAuthScheme selectedAuthScheme = new SelectedAuthScheme<>( CompletableFuture.completedFuture(identity), - signer, + httpSigner, AuthSchemeOption.builder() .schemeId("my.auth#myAuth") .putSignerProperty(SIGNER_PROPERTY, "value") @@ -267,7 +264,7 @@ public void execute_selectedAuthScheme_asyncRequestBody_doesSraSignAsync() throw SdkHttpRequest signedRequest = ValidSdkObjects.sdkHttpFullRequest().build(); Publisher signedPayload = AsyncRequestBody.fromString("signed async request body"); - when(signer.signAsync(Mockito.>any())) + when(httpSigner.signAsync(ArgumentMatchers.>any())) .thenReturn( CompletableFuture.completedFuture(AsyncSignedRequest.builder() .request(signedRequest) @@ -284,7 +281,7 @@ public void execute_selectedAuthScheme_asyncRequestBody_doesSraSignAsync() throw assertThat(context.requestProvider()).isSameAs(signedPayload); // assert that the input to the signer is as expected, including that signer properties are set - verify(signer).signAsync(asyncSignRequestCaptor.capture()); + verify(httpSigner).signAsync(asyncSignRequestCaptor.capture()); AsyncSignRequest signRequest = asyncSignRequestCaptor.getValue(); assertThat(signRequest.identity()).isSameAs(identity); assertThat(signRequest.request()).isSameAs(request); @@ -303,12 +300,12 @@ public void execute_selectedAuthScheme_asyncRequestBody_doesSraSignAsync() throw } @Test - public void execute_selectedNoAuthAuthScheme_doesSraSignAsync() throws Exception { + public void execute_selectedNoAuthAuthScheme_nullSigner_doesSraSign() throws Exception { AsyncRequestBody asyncPayload = AsyncRequestBody.fromString("async request body"); // Set up a scheme with smithy.api#noAuth SelectedAuthScheme selectedAuthScheme = new SelectedAuthScheme<>( CompletableFuture.completedFuture(identity), - signer, + httpSigner, AuthSchemeOption.builder() .schemeId("smithy.api#noAuth") .putSignerProperty(SIGNER_PROPERTY, "value") @@ -317,7 +314,7 @@ public void execute_selectedNoAuthAuthScheme_doesSraSignAsync() throws Exception SdkHttpRequest signedRequest = ValidSdkObjects.sdkHttpFullRequest().build(); Publisher signedPayload = AsyncRequestBody.fromString("signed async request body"); - when(signer.signAsync(Mockito.>any())) + when(httpSigner.signAsync(ArgumentMatchers.>any())) .thenReturn( CompletableFuture.completedFuture(AsyncSignedRequest.builder() .request(signedRequest) @@ -334,7 +331,7 @@ public void execute_selectedNoAuthAuthScheme_doesSraSignAsync() throws Exception assertThat(context.requestProvider()).isSameAs(signedPayload); // assert that the input to the signer is as expected, including that signer properties are set - verify(signer).signAsync(asyncSignRequestCaptor.capture()); + verify(httpSigner).signAsync(asyncSignRequestCaptor.capture()); AsyncSignRequest signRequest = asyncSignRequestCaptor.getValue(); assertThat(signRequest.identity()).isSameAs(identity); assertThat(signRequest.request()).isSameAs(request); @@ -353,7 +350,7 @@ public void execute_selectedNoAuthAuthScheme_doesSraSignAsync() throws Exception } @Test - public void execute_noSelectedAuthScheme_doesPreSraSign() throws Exception { + public void execute_nullSelectedAuthScheme_signer_doesPreSraSign() throws Exception { RequestExecutionContext context = createContext(null, oldSigner); SdkHttpFullRequest request = ValidSdkObjects.sdkHttpFullRequest().build(); @@ -371,11 +368,11 @@ public void execute_noSelectedAuthScheme_doesPreSraSign() throws Exception { // assert that metrics are collected verify(metricCollector).reportMetric(eq(SIGNING_DURATION), any()); - verifyNoInteractions(signer); + verifyNoInteractions(httpSigner); } @Test - public void execute_noSelectedAuthScheme_AsyncRequestBodySigner_doesPreSraSignAsyncRequestBody() throws Exception { + public void execute_nullSelectedAuthScheme_AsyncRequestBodySigner_doesPreSraSignAsyncRequestBody() throws Exception { AsyncRequestBody asyncPayload = AsyncRequestBody.fromString("async request body"); AsyncRequestBody signedPayload = AsyncRequestBody.fromString("signed async request body"); @@ -404,11 +401,11 @@ public void execute_noSelectedAuthScheme_AsyncRequestBodySigner_doesPreSraSignAs // assert that metrics are collected verify(metricCollector).reportMetric(eq(SIGNING_DURATION), any()); - verifyNoInteractions(signer); + verifyNoInteractions(httpSigner); } @Test - public void execute_noSelectedAuthScheme_AsyncSigner_doesPreSraSignAsync() throws Exception { + public void execute_nullSelectedAuthScheme_AsyncSigner_doesPreSraSignAsync() throws Exception { AsyncRequestBody asyncPayload = AsyncRequestBody.fromString("async request body"); TestAsyncSigner asyncSigner = mock(TestAsyncSigner.class); @@ -441,13 +438,12 @@ public void execute_noSelectedAuthScheme_AsyncSigner_doesPreSraSignAsync() throw // assert that metrics are collected verify(metricCollector).reportMetric(eq(SIGNING_DURATION), any()); - verifyNoInteractions(signer); + verifyNoInteractions(httpSigner); } @Test - public void execute_noSelectedAuthScheme_nullSigner_skipsSigning() throws Exception { - Signer oldSigner = null; - RequestExecutionContext context = createContext(null, oldSigner); + public void execute_nullSelectedAuthScheme_nullSigner_skipsSigning() throws Exception { + RequestExecutionContext context = createContext(null, null); SdkHttpFullRequest request = ValidSdkObjects.sdkHttpFullRequest().build(); SdkHttpFullRequest result = stage.execute(request, context).join(); @@ -460,11 +456,11 @@ public void execute_noSelectedAuthScheme_nullSigner_skipsSigning() throws Except verifyNoInteractions(metricCollector); - verifyNoInteractions(signer); + verifyNoInteractions(httpSigner); } @Test - public void execute_noSelectedAuthScheme_signerUsesTimeOffset() throws Exception { + public void execute_nullSelectedAuthScheme_signer_usesTimeOffset() throws Exception { httpClientDependencies.updateTimeOffset(100); RequestExecutionContext context = createContext(null, oldSigner); @@ -484,18 +480,18 @@ public void execute_noSelectedAuthScheme_signerUsesTimeOffset() throws Exception // assert that metrics are collected verify(metricCollector).reportMetric(eq(SIGNING_DURATION), any()); - verifyNoInteractions(signer); + verifyNoInteractions(httpSigner); } @Test - public void execute_selectedAuthScheme_OldSignerOverridden_doesPreSraSign() throws Exception { + public void execute_selectedAuthScheme_signer_doesPreSraSign() throws Exception { // Set up a scheme SelectedAuthScheme selectedAuthScheme = new SelectedAuthScheme<>( CompletableFuture.completedFuture(identity), - signer, + httpSigner, AuthSchemeOption.builder().schemeId("my.auth#myAuth").build()); - RequestExecutionContext context = createContext(selectedAuthScheme, oldSigner, true); + RequestExecutionContext context = createContext(selectedAuthScheme, oldSigner); SdkHttpFullRequest request = ValidSdkObjects.sdkHttpFullRequest().build(); @@ -512,49 +508,7 @@ public void execute_selectedAuthScheme_OldSignerOverridden_doesPreSraSign() thro // assert that metrics are collected verify(metricCollector).reportMetric(eq(SIGNING_DURATION), any()); - verifyNoInteractions(signer); - } - - @Test - public void execute_selectedAuthScheme_OldSignerRequestOverridden_doesPreSraSign() throws Exception { - // Set up a scheme - SelectedAuthScheme selectedAuthScheme = new SelectedAuthScheme<>( - CompletableFuture.completedFuture(identity), - signer, - AuthSchemeOption.builder().schemeId("my.auth#myAuth").build()); - - SdkRequest sdkRequest = - NoopTestRequest.builder() - .overrideConfiguration(SdkRequestOverrideConfiguration.builder().signer(oldSigner).build()) - .build(); - - RequestExecutionContext context = createContext(selectedAuthScheme, oldSigner, false, sdkRequest); - - SdkHttpFullRequest request = ValidSdkObjects.sdkHttpFullRequest().build(); - - SdkHttpFullRequest signedRequest = ValidSdkObjects.sdkHttpFullRequest().build(); - // Creating a copy because original executionAttributes may be directly mutated by SigningStage, e.g., timeOffset - when(oldSigner.sign(request, context.executionAttributes().copy().putAttribute(TIME_OFFSET, 0))).thenReturn(signedRequest); - - SdkHttpFullRequest result = stage.execute(request, context).join(); - - assertThat(result).isSameAs(signedRequest); - // assert that interceptor context is updated with result - assertThat(context.executionContext().interceptorContext().httpRequest()).isSameAs(result); - - // assert that metrics are collected - verify(metricCollector).reportMetric(eq(SIGNING_DURATION), any()); - - verifyNoInteractions(signer); - } - - private RequestExecutionContext createContext(SelectedAuthScheme selectedAuthScheme) { - return createContext(selectedAuthScheme, oldSigner); - } - - private RequestExecutionContext createContext(SelectedAuthScheme selectedAuthScheme, - AsyncRequestBody requestProvider) { - return createContext(selectedAuthScheme, requestProvider, oldSigner); + verifyNoInteractions(httpSigner); } private RequestExecutionContext createContext(SelectedAuthScheme selectedAuthScheme, Signer oldSigner) { @@ -564,27 +518,7 @@ private RequestExecutionContext createContext(SelectedAuthScheme selec private RequestExecutionContext createContext(SelectedAuthScheme selectedAuthScheme, AsyncRequestBody requestProvider, Signer oldSigner) { - return createContext(selectedAuthScheme, requestProvider, oldSigner, false, ValidSdkObjects.sdkRequest()); - } - - private RequestExecutionContext createContext(SelectedAuthScheme selectedAuthScheme, - Signer oldSigner, - boolean isSignerOverridden) { - return createContext(selectedAuthScheme, oldSigner, isSignerOverridden, ValidSdkObjects.sdkRequest()); - } - - private RequestExecutionContext createContext(SelectedAuthScheme selectedAuthScheme, - Signer oldSigner, - boolean isSignerOverridden, - SdkRequest sdkRequest) { - return createContext(selectedAuthScheme, null, oldSigner, isSignerOverridden, sdkRequest); - } - - private RequestExecutionContext createContext(SelectedAuthScheme selectedAuthScheme, - AsyncRequestBody requestProvider, - Signer oldSigner, - boolean isSignerOverridden, - SdkRequest sdkRequest) { + SdkRequest sdkRequest = ValidSdkObjects.sdkRequest(); InterceptorContext interceptorContext = InterceptorContext.builder() .request(sdkRequest) @@ -595,7 +529,6 @@ private RequestExecutionContext createContext(SelectedAuthScheme selec ExecutionAttributes executionAttributes = ExecutionAttributes.builder() .put(SELECTED_AUTH_SCHEME, selectedAuthScheme) - .put(SIGNER_OVERRIDDEN, isSignerOverridden) .build(); ExecutionContext executionContext = ExecutionContext.builder() diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/http/pipeline/stages/SigningStageTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/http/pipeline/stages/SigningStageTest.java index 01ecbc568a22..25809b32794a 100644 --- a/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/http/pipeline/stages/SigningStageTest.java +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/http/pipeline/stages/SigningStageTest.java @@ -22,7 +22,6 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; -import static software.amazon.awssdk.core.interceptor.SdkExecutionAttribute.SIGNER_OVERRIDDEN; import static software.amazon.awssdk.core.interceptor.SdkExecutionAttribute.TIME_OFFSET; import static software.amazon.awssdk.core.interceptor.SdkInternalExecutionAttribute.SELECTED_AUTH_SCHEME; import static software.amazon.awssdk.core.metrics.CoreMetric.SIGNING_DURATION; @@ -36,16 +35,14 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; +import org.mockito.ArgumentMatchers; import org.mockito.Captor; import org.mockito.Mock; -import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; import software.amazon.awssdk.core.SdkRequest; -import software.amazon.awssdk.core.SdkRequestOverrideConfiguration; import software.amazon.awssdk.core.SelectedAuthScheme; import software.amazon.awssdk.core.client.config.SdkClientConfiguration; import software.amazon.awssdk.core.http.ExecutionContext; -import software.amazon.awssdk.core.http.NoopTestRequest; import software.amazon.awssdk.core.interceptor.ExecutionAttributes; import software.amazon.awssdk.core.interceptor.InterceptorContext; import software.amazon.awssdk.core.internal.http.HttpClientDependencies; @@ -55,9 +52,9 @@ import software.amazon.awssdk.http.SdkHttpRequest; 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.http.auth.spi.signer.SignRequest; import software.amazon.awssdk.http.auth.spi.signer.SignedRequest; +import software.amazon.awssdk.http.auth.spi.signer.SignerProperty; import software.amazon.awssdk.identity.spi.Identity; import software.amazon.awssdk.metrics.MetricCollector; import utils.ValidSdkObjects; @@ -72,7 +69,7 @@ public class SigningStageTest { private Identity identity; @Mock - private HttpSigner signer; + private HttpSigner httpSigner; @Mock private Signer oldSigner; @@ -98,11 +95,11 @@ public void setup() { } @Test - public void execute_selectedAuthScheme_doesSraSign() throws Exception { + public void execute_selectedAuthScheme_nullSigner_doesSraSign() throws Exception { // Set up a scheme with a signer property SelectedAuthScheme selectedAuthScheme = new SelectedAuthScheme<>( CompletableFuture.completedFuture(identity), - signer, + httpSigner, AuthSchemeOption.builder() .schemeId("my.auth#myAuth") .putSignerProperty(SIGNER_PROPERTY, "value") @@ -110,7 +107,7 @@ public void execute_selectedAuthScheme_doesSraSign() throws Exception { RequestExecutionContext context = createContext(selectedAuthScheme, null); SdkHttpRequest signedRequest = ValidSdkObjects.sdkHttpFullRequest().build(); - when(signer.sign(Mockito.>any())) + when(httpSigner.sign(ArgumentMatchers.>any())) .thenReturn(SignedRequest.builder() .request(signedRequest) .build()); @@ -123,7 +120,7 @@ public void execute_selectedAuthScheme_doesSraSign() throws Exception { assertThat(context.executionContext().interceptorContext().httpRequest()).isSameAs(result); // assert that the input to the signer is as expected, including that signer properties are set - verify(signer).sign(signRequestCaptor.capture()); + verify(httpSigner).sign(signRequestCaptor.capture()); SignRequest signRequest = signRequestCaptor.getValue(); assertThat(signRequest.identity()).isSameAs(identity); assertThat(signRequest.request()).isSameAs(request); @@ -139,11 +136,11 @@ public void execute_selectedAuthScheme_doesSraSign() throws Exception { } @Test - public void execute_selectedAuthSchemeAndTimeOffsetSet_doesSraSignAndAdjustTheSigningClock() throws Exception { + public void execute_selectedAuthScheme_nullSigner_timeOffsetSet_doesSraSignAndAdjustTheSigningClock() throws Exception { // Set up a scheme with a signer property SelectedAuthScheme selectedAuthScheme = new SelectedAuthScheme<>( CompletableFuture.completedFuture(identity), - signer, + httpSigner, AuthSchemeOption.builder() .schemeId("my.auth#myAuth") .putSignerProperty(SIGNER_PROPERTY, "value") @@ -154,7 +151,7 @@ public void execute_selectedAuthSchemeAndTimeOffsetSet_doesSraSignAndAdjustTheSi httpClientDependencies.updateTimeOffset(TEST_TIME_OFFSET); SdkHttpRequest signedRequest = ValidSdkObjects.sdkHttpFullRequest().build(); - when(signer.sign(Mockito.>any())) + when(httpSigner.sign(ArgumentMatchers.>any())) .thenReturn(SignedRequest.builder() .request(signedRequest) .build()); @@ -167,7 +164,7 @@ public void execute_selectedAuthSchemeAndTimeOffsetSet_doesSraSignAndAdjustTheSi assertThat(context.executionContext().interceptorContext().httpRequest()).isSameAs(result); // Assert that the input to the signer is as expected, including that signer properties are set - verify(signer).sign(signRequestCaptor.capture()); + verify(httpSigner).sign(signRequestCaptor.capture()); SignRequest signRequest = signRequestCaptor.getValue(); assertThat(signRequest.identity()).isSameAs(identity); assertThat(signRequest.request()).isSameAs(request); @@ -186,22 +183,22 @@ public void execute_selectedAuthSchemeAndTimeOffsetSet_doesSraSignAndAdjustTheSi } @Test - public void execute_selectedAuthScheme_doesSraSignAndDoesNotOverrideAuthSchemeOptionClock() throws Exception { + public void execute_selectedAuthScheme_nullSigner_doesSraSignAndDoesNotOverrideAuthSchemeOptionClock() throws Exception { // Set up a scheme with a signer property and the signing clock set Clock clock = testClock(); SelectedAuthScheme selectedAuthScheme = new SelectedAuthScheme<>( CompletableFuture.completedFuture(identity), - signer, + httpSigner, AuthSchemeOption.builder() .schemeId("my.auth#myAuth") .putSignerProperty(SIGNER_PROPERTY, "value") // The auth scheme option includes the signing clock property .putSignerProperty(HttpSigner.SIGNING_CLOCK, clock) .build()); - RequestExecutionContext context = createContext(selectedAuthScheme); + RequestExecutionContext context = createContext(selectedAuthScheme, null); SdkHttpRequest signedRequest = ValidSdkObjects.sdkHttpFullRequest().build(); - when(signer.sign(Mockito.>any())) + when(httpSigner.sign(ArgumentMatchers.>any())) .thenReturn(SignedRequest.builder() .request(signedRequest) .build()); @@ -214,7 +211,7 @@ public void execute_selectedAuthScheme_doesSraSignAndDoesNotOverrideAuthSchemeOp assertThat(context.executionContext().interceptorContext().httpRequest()).isSameAs(result); // assert that the input to the signer is as expected, including that signer properties are set - verify(signer).sign(signRequestCaptor.capture()); + verify(httpSigner).sign(signRequestCaptor.capture()); SignRequest signRequest = signRequestCaptor.getValue(); assertThat(signRequest.identity()).isSameAs(identity); assertThat(signRequest.request()).isSameAs(request); @@ -231,18 +228,18 @@ public void execute_selectedAuthScheme_doesSraSignAndDoesNotOverrideAuthSchemeOp } @Test - public void execute_selectedNoAuthAuthScheme_doesSraSign() throws Exception { + public void execute_selectedNoAuthAuthScheme_nullSigner_doesSraSign() throws Exception { // Set up a scheme with smithy.api#noAuth SelectedAuthScheme selectedAuthScheme = new SelectedAuthScheme<>( CompletableFuture.completedFuture(identity), - signer, + httpSigner, AuthSchemeOption.builder() .schemeId("smithy.api#noAuth") .build()); RequestExecutionContext context = createContext(selectedAuthScheme, null); SdkHttpRequest signedRequest = ValidSdkObjects.sdkHttpFullRequest().build(); - when(signer.sign(Mockito.>any())) + when(httpSigner.sign(ArgumentMatchers.>any())) .thenReturn(SignedRequest.builder() .request(signedRequest) .build()); @@ -255,7 +252,7 @@ public void execute_selectedNoAuthAuthScheme_doesSraSign() throws Exception { assertThat(context.executionContext().interceptorContext().httpRequest()).isSameAs(result); // assert that the input to the signer is as expected, including that signer properties are set - verify(signer).sign(signRequestCaptor.capture()); + verify(httpSigner).sign(signRequestCaptor.capture()); SignRequest signRequest = signRequestCaptor.getValue(); assertThat(signRequest.identity()).isSameAs(identity); assertThat(signRequest.request()).isSameAs(request); @@ -273,7 +270,7 @@ public void execute_selectedNoAuthAuthScheme_doesSraSign() throws Exception { } @Test - public void execute_noSelectedAuthScheme_doesPreSraSign() throws Exception { + public void execute_nullSelectedAuthScheme_signer_doesPreSraSign() throws Exception { RequestExecutionContext context = createContext(null, oldSigner); SdkHttpFullRequest request = ValidSdkObjects.sdkHttpFullRequest().build(); @@ -291,13 +288,12 @@ public void execute_noSelectedAuthScheme_doesPreSraSign() throws Exception { // assert that metrics are collected verify(metricCollector).reportMetric(eq(SIGNING_DURATION), any()); - verifyNoInteractions(signer); + verifyNoInteractions(httpSigner); } @Test - public void execute_noSelectedAuthScheme_nullSigner_skipsSigning() throws Exception { - Signer oldSigner = null; - RequestExecutionContext context = createContext(null, oldSigner); + public void execute_nullSelectedAuthScheme_nullSigner_skipsSigning() throws Exception { + RequestExecutionContext context = createContext(null, null); SdkHttpFullRequest request = ValidSdkObjects.sdkHttpFullRequest().build(); SdkHttpFullRequest result = stage.execute(request, context); @@ -310,11 +306,11 @@ public void execute_noSelectedAuthScheme_nullSigner_skipsSigning() throws Except verifyNoInteractions(metricCollector); - verifyNoInteractions(signer); + verifyNoInteractions(httpSigner); } @Test - public void execute_noSelectedAuthScheme_signerUsesTimeOffset() throws Exception { + public void execute_nullSelectedAuthScheme_signer_usesTimeOffset() throws Exception { httpClientDependencies.updateTimeOffset(100); RequestExecutionContext context = createContext(null, oldSigner); @@ -334,18 +330,18 @@ public void execute_noSelectedAuthScheme_signerUsesTimeOffset() throws Exception // assert that metrics are collected verify(metricCollector).reportMetric(eq(SIGNING_DURATION), any()); - verifyNoInteractions(signer); + verifyNoInteractions(httpSigner); } @Test - public void execute_selectedAuthScheme_OldSignerOverridden_doesPreSraSign() throws Exception { + public void execute_selectedAuthScheme_signer_doesPreSraSign() throws Exception { // Set up a scheme SelectedAuthScheme selectedAuthScheme = new SelectedAuthScheme<>( CompletableFuture.completedFuture(identity), - signer, + httpSigner, AuthSchemeOption.builder().schemeId("my.auth#myAuth").build()); - RequestExecutionContext context = createContext(selectedAuthScheme, oldSigner, true); + RequestExecutionContext context = createContext(selectedAuthScheme, oldSigner); SdkHttpFullRequest request = ValidSdkObjects.sdkHttpFullRequest().build(); @@ -362,60 +358,11 @@ public void execute_selectedAuthScheme_OldSignerOverridden_doesPreSraSign() thro // assert that metrics are collected verify(metricCollector).reportMetric(eq(SIGNING_DURATION), any()); - verifyNoInteractions(signer); - } - - @Test - public void execute_selectedAuthScheme_OldSignerRequestOverridden_doesPreSraSign() throws Exception { - // Set up a scheme - SelectedAuthScheme selectedAuthScheme = new SelectedAuthScheme<>( - CompletableFuture.completedFuture(identity), - signer, - AuthSchemeOption.builder().schemeId("my.auth#myAuth").build()); - - SdkRequest sdkRequest = - NoopTestRequest.builder() - .overrideConfiguration(SdkRequestOverrideConfiguration.builder().signer(oldSigner).build()) - .build(); - - RequestExecutionContext context = createContext(selectedAuthScheme, oldSigner, false, sdkRequest); - - SdkHttpFullRequest request = ValidSdkObjects.sdkHttpFullRequest().build(); - - SdkHttpFullRequest signedRequest = ValidSdkObjects.sdkHttpFullRequest().build(); - // Creating a copy because original executionAttributes may be directly mutated by SigningStage, e.g., timeOffset - when(oldSigner.sign(request, context.executionAttributes().copy().putAttribute(TIME_OFFSET, 0))).thenReturn(signedRequest); - - SdkHttpFullRequest result = stage.execute(request, context); - - assertThat(result).isSameAs(signedRequest); - // assert that interceptor context is updated with result - assertThat(context.executionContext().interceptorContext().httpRequest()).isSameAs(result); - - // assert that metrics are collected - verify(metricCollector).reportMetric(eq(SIGNING_DURATION), any()); - - verifyNoInteractions(signer); - } - - private RequestExecutionContext createContext(SelectedAuthScheme selectedAuthScheme) { - return createContext(selectedAuthScheme, oldSigner); + verifyNoInteractions(httpSigner); } private RequestExecutionContext createContext(SelectedAuthScheme selectedAuthScheme, Signer oldSigner) { - return createContext(selectedAuthScheme, oldSigner, false); - } - - private RequestExecutionContext createContext(SelectedAuthScheme selectedAuthScheme, - Signer oldSigner, - boolean isSignerOverridden) { - return createContext(selectedAuthScheme, oldSigner, isSignerOverridden, ValidSdkObjects.sdkRequest()); - } - - private RequestExecutionContext createContext(SelectedAuthScheme selectedAuthScheme, - Signer oldSigner, - boolean isSignerOverridden, - SdkRequest sdkRequest) { + SdkRequest sdkRequest = ValidSdkObjects.sdkRequest(); InterceptorContext interceptorContext = InterceptorContext.builder() .request(sdkRequest) @@ -426,7 +373,6 @@ private RequestExecutionContext createContext(SelectedAuthScheme selec ExecutionAttributes executionAttributes = ExecutionAttributes.builder() .put(SELECTED_AUTH_SCHEME, selectedAuthScheme) - .put(SIGNER_OVERRIDDEN, isSignerOverridden) .build(); ExecutionContext executionContext = ExecutionContext.builder()