Remove uses of legacy singer attributes (#4914)

sugmanue · web-flow · commit 368321f83d81 · 2024-02-12T08:46:33.000-08:00
diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/plugins/S3OverrideAuthSchemePropertiesPlugin.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/plugins/S3OverrideAuthSchemePropertiesPlugin.java
@@ -121,7 +121,6 @@ private <T> void putSingerProperty(AuthSchemeOption.Builder builder, SignerPrope
         builder.putSignerProperty((SignerProperty<T>) key, (T) value);
     }
 
-
     private boolean addConfiguredProperties(AuthSchemeOption option, S3AuthSchemeParams params) {
         String schemeId = option.schemeId();
         // We check here that the scheme id is sigV4 or sigV4a or some other in the same family.
diff --git a/services/s3control/src/it/java/software.amazon.awssdk.services.s3control/S3MrapIntegrationTest.java b/services/s3control/src/it/java/software.amazon.awssdk.services.s3control/S3MrapIntegrationTest.java
@@ -22,21 +22,18 @@
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.time.Duration;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
-import java.util.Optional;
 import java.util.stream.Stream;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
 import software.amazon.awssdk.auth.signer.AwsSignerExecutionAttribute;
-import software.amazon.awssdk.auth.signer.S3SignerExecutionAttribute;
 import software.amazon.awssdk.auth.signer.internal.SignerConstant;
 import software.amazon.awssdk.awscore.presigner.PresignedRequest;
-import software.amazon.awssdk.core.SdkRequest;
+import software.amazon.awssdk.core.SdkPlugin;
 import software.amazon.awssdk.core.interceptor.Context;
 import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
 import software.amazon.awssdk.core.interceptor.ExecutionInterceptor;
@@ -46,16 +43,14 @@
 import software.amazon.awssdk.core.waiters.WaiterAcceptor;
 import software.amazon.awssdk.http.HttpExecuteRequest;
 import software.amazon.awssdk.http.HttpExecuteResponse;
-import software.amazon.awssdk.http.SdkHttpMethod;
 import software.amazon.awssdk.http.SdkHttpRequest;
 import software.amazon.awssdk.http.apache.ApacheHttpClient;
 import software.amazon.awssdk.regions.Region;
 import software.amazon.awssdk.services.s3.S3Client;
 import software.amazon.awssdk.services.s3.S3Configuration;
+import software.amazon.awssdk.services.s3.internal.plugins.S3OverrideAuthSchemePropertiesPlugin;
 import software.amazon.awssdk.services.s3.model.BucketAlreadyOwnedByYouException;
 import software.amazon.awssdk.services.s3.model.NoSuchKeyException;
-import software.amazon.awssdk.services.s3.model.PutObjectRequest;
-import software.amazon.awssdk.services.s3.model.UploadPartRequest;
 import software.amazon.awssdk.services.s3.presigner.S3Presigner;
 import software.amazon.awssdk.services.s3.presigner.model.PresignedGetObjectRequest;
 import software.amazon.awssdk.services.s3control.model.BucketAlreadyExistsException;
@@ -99,7 +94,7 @@ public static void setupFixture() {
                                    .build();
 
         s3Client = mrapEnabledS3Client(Collections.singletonList(captureInterceptor));
-        s3ClientWithPayloadSigning = mrapEnabledS3Client(Arrays.asList(captureInterceptor, new PayloadSigningInterceptor()));
+        s3ClientWithPayloadSigning = mrapEnabledS3ClientWithPayloadSigning(captureInterceptor);
 
         stsClient = StsClient.builder()
                              .credentialsProvider(CREDENTIALS_PROVIDER_CHAIN)
@@ -309,6 +304,25 @@ private static S3Client mrapEnabledS3Client(List<ExecutionInterceptor> execution
                        .build();
     }
 
+    private static S3Client mrapEnabledS3ClientWithPayloadSigning(ExecutionInterceptor executionInterceptor) {
+        // We can't use here `S3OverrideAuthSchemePropertiesPlugin.enablePayloadSigningPlugin()` since
+        // it enables payload signing for *all* operations.
+        SdkPlugin plugin = S3OverrideAuthSchemePropertiesPlugin.builder()
+                                                               .payloadSigningEnabled(true)
+                                                               .addOperationConstraint("UploadPart")
+                                                               .addOperationConstraint("PutObject")
+                                                               .build();
+        return S3Client.builder()
+                       .region(REGION)
+                       .credentialsProvider(CREDENTIALS_PROVIDER_CHAIN)
+                       .serviceConfiguration(S3Configuration.builder()
+                                                            .useArnRegionEnabled(true)
+                                                            .build())
+                       .overrideConfiguration(o -> o.addExecutionInterceptor(executionInterceptor))
+                       .addPlugin(plugin)
+                       .build();
+    }
+
     private void deleteObjectIfExists(S3Client s31, String bucket1, String key) {
         System.out.println(bucket1);
         try {
@@ -341,21 +355,4 @@ public void beforeTransmission(Context.BeforeTransmission context, ExecutionAttr
             this.normalizePath = executionAttributes.getAttribute(AwsSignerExecutionAttribute.SIGNER_NORMALIZE_PATH);
         }
     }
-
-    private static class PayloadSigningInterceptor implements ExecutionInterceptor {
-
-        public Optional<RequestBody> modifyHttpContent(Context.ModifyHttpRequest context,
-                                                       ExecutionAttributes executionAttributes) {
-            SdkRequest sdkRequest = context.request();
-
-            if (sdkRequest instanceof PutObjectRequest || sdkRequest instanceof UploadPartRequest) {
-                executionAttributes.putAttribute(S3SignerExecutionAttribute.ENABLE_PAYLOAD_SIGNING, true);
-            }
-            if (!context.requestBody().isPresent() && context.httpRequest().method().equals(SdkHttpMethod.POST)) {
-                return Optional.of(RequestBody.fromBytes(new byte[0]));
-            }
-
-            return context.requestBody();
-        }
-    }
 }
diff --git a/services/s3control/src/main/java/software/amazon/awssdk/services/s3control/internal/interceptors/PayloadSigningInterceptor.java b/services/s3control/src/main/java/software/amazon/awssdk/services/s3control/internal/interceptors/PayloadSigningInterceptor.java
@@ -17,27 +17,25 @@
 
 import java.util.Optional;
 import software.amazon.awssdk.annotations.SdkInternalApi;
-import software.amazon.awssdk.auth.signer.S3SignerExecutionAttribute;
 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.sync.RequestBody;
 import software.amazon.awssdk.http.SdkHttpMethod;
 
 /**
- * Turns on payload signing and prevents moving query params to body during a POST which S3 doesn't like.
+ * Prevents moving query params to body during a POST which S3 doesn't like.
  */
 @SdkInternalApi
 public class PayloadSigningInterceptor implements ExecutionInterceptor {
 
     @Override
     public Optional<RequestBody> modifyHttpContent(Context.ModifyHttpRequest context,
                                                    ExecutionAttributes executionAttributes) {
-        executionAttributes.putAttribute(S3SignerExecutionAttribute.ENABLE_PAYLOAD_SIGNING, true);
-        if (!context.requestBody().isPresent() && context.httpRequest().method() == SdkHttpMethod.POST) {
+        Optional<RequestBody> bodyOptional = context.requestBody();
+        if (context.httpRequest().method() == SdkHttpMethod.POST && !bodyOptional.isPresent()) {
             return Optional.of(RequestBody.fromBytes(new byte[0]));
         }
-
-        return context.requestBody();
+        return bodyOptional;
     }
 }
diff --git a/services/s3control/src/test/java/software/amazon/awssdk/services/s3control/internal/interceptors/PayloadSigningInterceptorTest.java b/services/s3control/src/test/java/software/amazon/awssdk/services/s3control/internal/interceptors/PayloadSigningInterceptorTest.java
@@ -19,7 +19,6 @@
 import java.util.Optional;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
-import software.amazon.awssdk.auth.signer.S3SignerExecutionAttribute;
 import software.amazon.awssdk.core.Protocol;
 import software.amazon.awssdk.core.SdkRequest;
 import software.amazon.awssdk.core.async.AsyncRequestBody;
@@ -53,7 +52,6 @@ public void modifyHttpContent_AddsExecutionAttributeAndPayload() {
 
         assertThat(modified.isPresent()).isTrue();
         assertThat(modified.get().contentLength()).isEqualTo(0);
-        assertThat(executionAttributes.getAttribute(S3SignerExecutionAttribute.ENABLE_PAYLOAD_SIGNING)).isTrue();
     }
 
     @Test
@@ -65,7 +63,6 @@ public void modifyHttpContent_DoesNotReplaceBody() {
 
         assertThat(modified.isPresent()).isTrue();
         assertThat(modified.get().contentLength()).isEqualTo(5);
-        assertThat(executionAttributes.getAttribute(S3SignerExecutionAttribute.ENABLE_PAYLOAD_SIGNING)).isTrue();
     }
 
     public final class Context implements software.amazon.awssdk.core.interceptor.Context.ModifyHttpRequest {

Original file line number	Diff line number	Diff line change
`@@ -121,7 +121,6 @@ private <T> void putSingerProperty(AuthSchemeOption.Builder builder, SignerPrope`
`121`	`121`	`builder.putSignerProperty((SignerProperty<T>) key, (T) value);`
`122`	`122`	`}`
`123`	`123`
`124`		`-`
`125`	`124`	`private boolean addConfiguredProperties(AuthSchemeOption option, S3AuthSchemeParams params) {`
`126`	`125`	`String schemeId = option.schemeId();`
`127`	`126`	`// We check here that the scheme id is sigV4 or sigV4a or some other in the same family.`