feat: Added support for object canned access control lists. (#9)

msailes · web-flow · commit 9abc1026afab · 2020-10-16T10:52:38.000-04:00
feat: Added support for object canned access control lists.
diff --git a/README.md b/README.md
@@ -24,7 +24,7 @@ You can download release builds through the [releases section of this](https://g
   <dependency>
     <groupId>software.amazon.payloadoffloading</groupId>
     <artifactId>payloadoffloading-common</artifactId>
-    <version>2.0.0</version>
+    <version>2.1.0</version>
   </dependency>
 ```   
 
diff --git a/pom.xml b/pom.xml
@@ -6,7 +6,7 @@
 
     <groupId>software.amazon.payloadoffloading</groupId>
     <artifactId>payloadoffloading-common</artifactId>
-    <version>2.0.0</version>
+    <version>2.1.0</version>
     <packaging>jar</packaging>
     <name>Payload offloading common library for AWS</name>
     <description>Common library between extended Amazon AWS clients to save payloads up to 2GB on Amazon S3.</description>
diff --git a/src/main/java/software/amazon/payloadoffloading/PayloadStorageConfiguration.java b/src/main/java/software/amazon/payloadoffloading/PayloadStorageConfiguration.java
@@ -5,6 +5,7 @@
 import software.amazon.awssdk.annotations.NotThreadSafe;
 import software.amazon.awssdk.core.exception.SdkClientException;
 import software.amazon.awssdk.services.s3.S3Client;
+import software.amazon.awssdk.services.s3.model.ObjectCannedACL;
 
 /**
  * <p>Amazon payload storage configuration options such as Amazon S3 client,
@@ -42,11 +43,16 @@ public class PayloadStorageConfiguration {
      * This field is optional, it is set only when we want to configure S3 Server Side Encryption with KMS.
      */
     private ServerSideEncryptionStrategy serverSideEncryptionStrategy;
+    /**
+     * This field is optional, it is set only when we want to add access control list to Amazon S3 buckets and objects
+     */
+    private ObjectCannedACL objectCannedACL;
 
     public PayloadStorageConfiguration() {
         s3 = null;
         s3BucketName = null;
         serverSideEncryptionStrategy = null;
+        objectCannedACL = null;
     }
 
     public PayloadStorageConfiguration(PayloadStorageConfiguration other) {
@@ -56,6 +62,7 @@ public PayloadStorageConfiguration(PayloadStorageConfiguration other) {
         this.alwaysThroughS3 = other.isAlwaysThroughS3();
         this.payloadSizeThreshold = other.getPayloadSizeThreshold();
         this.serverSideEncryptionStrategy = other.getServerSideEncryptionStrategy();
+        this.objectCannedACL = other.getObjectCannedACL();
     }
 
     /**
@@ -235,4 +242,38 @@ public ServerSideEncryptionStrategy getServerSideEncryptionStrategy() {
         return this.serverSideEncryptionStrategy;
     }
 
+    /**
+     * Configures the ACL to apply to the Amazon S3 putObject request.
+     * @param objectCannedACL
+     *            The ACL to be used when storing objects in Amazon S3
+     */
+    public void setObjectCannedACL(ObjectCannedACL objectCannedACL) {
+        this.objectCannedACL = objectCannedACL;
+    }
+
+    /**
+     * Configures the ACL to apply to the Amazon S3 putObject request.
+     * @param objectCannedACL
+     *            The ACL to be used when storing objects in Amazon S3
+     */
+    public PayloadStorageConfiguration withObjectCannedACL(ObjectCannedACL objectCannedACL) {
+        setObjectCannedACL(objectCannedACL);
+        return this;
+    }
+
+    /**
+     * Checks whether an ACL have been configured for storing objects in Amazon S3.
+     * @return True if ACL is defined
+     */
+    public boolean isObjectCannedACLDefined() {
+        return null != objectCannedACL;
+    }
+
+    /**
+     * Gets the AWS ACL to apply to the Amazon S3 putObject request.
+     * @return Amazon S3 object ACL
+     */
+    public ObjectCannedACL getObjectCannedACL() {
+        return objectCannedACL;
+    }
 }
diff --git a/src/main/java/software/amazon/payloadoffloading/S3BackedPayloadStore.java b/src/main/java/software/amazon/payloadoffloading/S3BackedPayloadStore.java
@@ -13,24 +13,17 @@ public class S3BackedPayloadStore implements PayloadStore {
 
     private final String s3BucketName;
     private final S3Dao s3Dao;
-    private final ServerSideEncryptionStrategy serverSideEncryptionStrategy;
 
     public S3BackedPayloadStore(S3Dao s3Dao, String s3BucketName) {
-        this(s3Dao, s3BucketName, null);
-    }
-
-    public S3BackedPayloadStore(S3Dao s3Dao, String s3BucketName, ServerSideEncryptionStrategy serverSideEncryptionStrategy) {
         this.s3BucketName = s3BucketName;
         this.s3Dao = s3Dao;
-        this.serverSideEncryptionStrategy = serverSideEncryptionStrategy;
     }
 
     @Override
     public String storeOriginalPayload(String payload) {
         String s3Key = UUID.randomUUID().toString();
 
-        // Store the payload content in S3.
-        s3Dao.storeTextInS3(s3BucketName, s3Key, serverSideEncryptionStrategy, payload);
+        s3Dao.storeTextInS3(s3BucketName, s3Key, payload);
         LOG.info("S3 object created, Bucket name: " + s3BucketName + ", Object key: " + s3Key + ".");
 
         // Convert S3 pointer (bucket name, key, etc) to JSON string
diff --git a/src/main/java/software/amazon/payloadoffloading/S3Dao.java b/src/main/java/software/amazon/payloadoffloading/S3Dao.java
@@ -10,6 +10,7 @@
 import software.amazon.awssdk.services.s3.model.DeleteObjectRequest;
 import software.amazon.awssdk.services.s3.model.GetObjectRequest;
 import software.amazon.awssdk.services.s3.model.GetObjectResponse;
+import software.amazon.awssdk.services.s3.model.ObjectCannedACL;
 import software.amazon.awssdk.services.s3.model.PutObjectRequest;
 import software.amazon.awssdk.utils.IoUtils;
 
@@ -21,9 +22,17 @@
 public class S3Dao {
     private static final Logger LOG = LoggerFactory.getLogger(S3Dao.class);
     private final S3Client s3Client;
+    private final ServerSideEncryptionStrategy serverSideEncryptionStrategy;
+    private final ObjectCannedACL objectCannedACL;
 
     public S3Dao(S3Client s3Client) {
+        this(s3Client, null, null);
+    }
+
+    public S3Dao(S3Client s3Client, ServerSideEncryptionStrategy serverSideEncryptionStrategy, ObjectCannedACL objectCannedACL) {
         this.s3Client = s3Client;
+        this.serverSideEncryptionStrategy = serverSideEncryptionStrategy;
+        this.objectCannedACL = objectCannedACL;
     }
 
     public String getTextFromS3(String s3BucketName, String s3Key) {
@@ -56,11 +65,15 @@ public String getTextFromS3(String s3BucketName, String s3Key) {
         return embeddedText;
     }
 
-    public void storeTextInS3(String s3BucketName, String s3Key, ServerSideEncryptionStrategy serverSideEncryptionStrategy, String payloadContentStr) {
+    public void storeTextInS3(String s3BucketName, String s3Key, String payloadContentStr) {
         PutObjectRequest.Builder putObjectRequestBuilder = PutObjectRequest.builder()
                 .bucket(s3BucketName)
                 .key(s3Key);
 
+        if (objectCannedACL != null) {
+            putObjectRequestBuilder.acl(objectCannedACL);
+        }
+
         // https://docs.aws.amazon.com/AmazonS3/latest/dev/kms-using-sdks.html
         if (serverSideEncryptionStrategy != null) {
             serverSideEncryptionStrategy.decorate(putObjectRequestBuilder);
diff --git a/src/test/java/software/amazon/payloadoffloading/PayloadStorageConfigurationTest.java b/src/test/java/software/amazon/payloadoffloading/PayloadStorageConfigurationTest.java
@@ -2,6 +2,7 @@
 
 import org.junit.Test;
 import software.amazon.awssdk.services.s3.S3Client;
+import software.amazon.awssdk.services.s3.model.ObjectCannedACL;
 
 import static org.mockito.Mockito.mock;
 import static org.junit.Assert.*;
@@ -12,8 +13,8 @@
 public class PayloadStorageConfigurationTest {
 
     private static final String s3BucketName = "test-bucket-name";
-    private static final String s3ServerSideEncryptionKMSKeyId = "test-customer-managed-kms-key-id";
     private static final ServerSideEncryptionStrategy SERVER_SIDE_ENCRYPTION_STRATEGY = ServerSideEncryptionFactory.awsManagedCmk();
+    private final ObjectCannedACL objectCannelACL = ObjectCannedACL.BUCKET_OWNER_FULL_CONTROL;
 
     @Test
     public void testCopyConstructor() {
@@ -27,14 +28,16 @@ public void testCopyConstructor() {
         payloadStorageConfiguration.withPayloadSupportEnabled(s3, s3BucketName)
                 .withAlwaysThroughS3(alwaysThroughS3)
                 .withPayloadSizeThreshold(payloadSizeThreshold)
-                .withServerSideEncryption(SERVER_SIDE_ENCRYPTION_STRATEGY);
+                .withServerSideEncryption(SERVER_SIDE_ENCRYPTION_STRATEGY)
+                .withObjectCannedACL(objectCannelACL);
 
         PayloadStorageConfiguration newPayloadStorageConfiguration = new PayloadStorageConfiguration(payloadStorageConfiguration);
 
         assertEquals(s3, newPayloadStorageConfiguration.getS3Client());
         assertEquals(s3BucketName, newPayloadStorageConfiguration.getS3BucketName());
         assertEquals(SERVER_SIDE_ENCRYPTION_STRATEGY, newPayloadStorageConfiguration.getServerSideEncryptionStrategy());
         assertTrue(newPayloadStorageConfiguration.isPayloadSupportEnabled());
+        assertEquals(objectCannelACL, newPayloadStorageConfiguration.getObjectCannedACL());
         assertEquals(alwaysThroughS3, newPayloadStorageConfiguration.isAlwaysThroughS3());
         assertEquals(payloadSizeThreshold, newPayloadStorageConfiguration.getPayloadSizeThreshold());
         assertNotSame(newPayloadStorageConfiguration, payloadStorageConfiguration);
@@ -80,4 +83,15 @@ public void testSseAwsKeyManagementParams() {
         payloadStorageConfiguration.setServerSideEncryptionStrategy(SERVER_SIDE_ENCRYPTION_STRATEGY);
         assertEquals(SERVER_SIDE_ENCRYPTION_STRATEGY, payloadStorageConfiguration.getServerSideEncryptionStrategy());
     }
+
+    @Test
+    public void testCannedAccessControlList() {
+        PayloadStorageConfiguration payloadStorageConfiguration = new PayloadStorageConfiguration();
+
+        assertFalse(payloadStorageConfiguration.isObjectCannedACLDefined());
+
+        payloadStorageConfiguration.withObjectCannedACL(objectCannelACL);
+        assertTrue(payloadStorageConfiguration.isObjectCannedACLDefined());
+        assertEquals(objectCannelACL, payloadStorageConfiguration.getObjectCannedACL());
+    }
 }
diff --git a/src/test/java/software/amazon/payloadoffloading/S3BackedPayloadStoreTest.java b/src/test/java/software/amazon/payloadoffloading/S3BackedPayloadStoreTest.java
@@ -1,7 +1,6 @@
 package software.amazon.payloadoffloading;
 
 import junitparams.JUnitParamsRunner;
-import junitparams.Parameters;
 import org.hamcrest.Matchers;
 import org.junit.Before;
 import org.junit.Rule;
@@ -11,8 +10,7 @@
 import org.mockito.ArgumentCaptor;
 import software.amazon.awssdk.core.exception.SdkClientException;
 import software.amazon.awssdk.core.exception.SdkException;
-
-import java.util.Objects;
+import software.amazon.awssdk.services.s3.model.ObjectCannedACL;
 
 import static org.junit.Assert.*;
 import static org.mockito.ArgumentMatchers.any;
@@ -21,13 +19,9 @@
 @RunWith(JUnitParamsRunner.class)
 public class S3BackedPayloadStoreTest {
     private static final String S3_BUCKET_NAME = "test-bucket-name";
-    private static final String S3_SERVER_SIDE_ENCRYPTION_KMS_KEY_ID = "test-customer-managed-kms-key-id";
-    private static final ServerSideEncryptionStrategy KMS_WITH_CUSTOMER_KEY = ServerSideEncryptionFactory.customerKey(S3_SERVER_SIDE_ENCRYPTION_KMS_KEY_ID);
-    private static final ServerSideEncryptionStrategy KMS_WITH_AWS_MANAGED_CMK = ServerSideEncryptionFactory.awsManagedCmk();
     private static final String ANY_PAYLOAD = "AnyPayload";
     private static final String ANY_S3_KEY = "AnyS3key";
     private static final String INCORRECT_POINTER_EXCEPTION_MSG = "Failed to read the S3 object pointer from given string";
-    private static final Long ANY_PAYLOAD_LENGTH = 300000L;
     private PayloadStore payloadStore;
     private S3Dao s3Dao;
 
@@ -40,63 +34,23 @@ public void setup() {
         payloadStore = new S3BackedPayloadStore(s3Dao, S3_BUCKET_NAME);
     }
 
-    private Object[] testData() {
-        // Here, we create separate mock of S3Dao because JUnitParamsRunner collects parameters
-        // for tests well before invocation of @Before or @BeforeClass methods.
-        // That means our default s3Dao mock isn't instantiated until then. For parameterized tests,
-        // we instantiate our local S3Dao mock per combination, pass it to S3BackedPayloadStore and also pass it
-        // as test parameter to allow verifying calls to the mockS3Dao.
-        S3Dao noEncryptionS3Dao = mock(S3Dao.class);
-        S3Dao defaultEncryptionS3Dao = mock(S3Dao.class);
-        S3Dao customerKMSKeyEncryptionS3Dao = mock(S3Dao.class);
-        return new Object[][]{
-                // No S3 SSE-KMS encryption
-                {
-                    new S3BackedPayloadStore(noEncryptionS3Dao, S3_BUCKET_NAME),
-                    null,
-                    noEncryptionS3Dao
-                },
-                // S3 SSE-KMS encryption with AWS managed KMS keys
-                {
-                    new S3BackedPayloadStore(defaultEncryptionS3Dao, S3_BUCKET_NAME, KMS_WITH_AWS_MANAGED_CMK),
-                    KMS_WITH_AWS_MANAGED_CMK,
-                    defaultEncryptionS3Dao
-                },
-                // S3 SSE-KMS encryption with customer managed KMS key
-                {
-                    new S3BackedPayloadStore(customerKMSKeyEncryptionS3Dao, S3_BUCKET_NAME, KMS_WITH_CUSTOMER_KEY),
-                    KMS_WITH_CUSTOMER_KEY,
-                    customerKMSKeyEncryptionS3Dao
-                }
-        };
-    }
-
     @Test
-    @Parameters(method = "testData")
-    public void testStoreOriginalPayloadOnSuccess(PayloadStore payloadStore, ServerSideEncryptionStrategy expectedParams, S3Dao mockS3Dao) {
+    public void testStoreOriginalPayloadOnSuccess() {
         String actualPayloadPointer = payloadStore.storeOriginalPayload(ANY_PAYLOAD);
 
         ArgumentCaptor<String> keyCaptor = ArgumentCaptor.forClass(String.class);
         ArgumentCaptor<ServerSideEncryptionStrategy> sseArgsCaptor = ArgumentCaptor.forClass(ServerSideEncryptionStrategy.class);
+        ArgumentCaptor<ObjectCannedACL> cannedArgsCaptor = ArgumentCaptor.forClass(ObjectCannedACL.class);
 
-        verify(mockS3Dao, times(1)).storeTextInS3(eq(S3_BUCKET_NAME), keyCaptor.capture(),
-                sseArgsCaptor.capture(), eq(ANY_PAYLOAD));
+        verify(s3Dao, times(1)).storeTextInS3(eq(S3_BUCKET_NAME), keyCaptor.capture(),
+               eq(ANY_PAYLOAD));
 
         PayloadS3Pointer expectedPayloadPointer = new PayloadS3Pointer(S3_BUCKET_NAME, keyCaptor.getValue());
         assertEquals(expectedPayloadPointer.toJson(), actualPayloadPointer);
-
-        if (expectedParams == null) {
-            assertTrue(sseArgsCaptor.getValue() == null);
-        } else {
-            assertEquals(expectedParams, sseArgsCaptor.getValue());
-        }
     }
 
     @Test
-    @Parameters(method = "testData")
-    public void testStoreOriginalPayloadDoesAlwaysCreateNewObjects(PayloadStore payloadStore,
-                                                                   ServerSideEncryptionStrategy expectedParams,
-                                                                   S3Dao mockS3Dao) {
+    public void testStoreOriginalPayloadDoesAlwaysCreateNewObjects() {
         //Store any payload
         String anyActualPayloadPointer = payloadStore.storeOriginalPayload(ANY_PAYLOAD);
 
@@ -105,9 +59,10 @@ public void testStoreOriginalPayloadDoesAlwaysCreateNewObjects(PayloadStore payl
 
         ArgumentCaptor<String> anyOtherKeyCaptor = ArgumentCaptor.forClass(String.class);
         ArgumentCaptor<ServerSideEncryptionStrategy> sseArgsCaptor = ArgumentCaptor.forClass(ServerSideEncryptionStrategy.class);
+        ArgumentCaptor<ObjectCannedACL> cannedArgsCaptor = ArgumentCaptor.forClass(ObjectCannedACL.class);
 
-        verify(mockS3Dao, times(2)).storeTextInS3(eq(S3_BUCKET_NAME), anyOtherKeyCaptor.capture(),
-                sseArgsCaptor.capture(), eq(ANY_PAYLOAD));
+        verify(s3Dao, times(2)).storeTextInS3(eq(S3_BUCKET_NAME), anyOtherKeyCaptor.capture(),
+                eq(ANY_PAYLOAD));
 
         String anyS3Key = anyOtherKeyCaptor.getAllValues().get(0);
         String anyOtherS3Key = anyOtherKeyCaptor.getAllValues().get(1);
@@ -120,25 +75,15 @@ public void testStoreOriginalPayloadDoesAlwaysCreateNewObjects(PayloadStore payl
 
         assertThat(anyS3Key, Matchers.not(anyOtherS3Key));
         assertThat(anyActualPayloadPointer, Matchers.not(anyOtherActualPayloadPointer));
-
-        if (expectedParams == null) {
-            assertTrue(sseArgsCaptor.getAllValues().stream().allMatch(Objects::isNull));
-        } else {
-            assertTrue(sseArgsCaptor.getAllValues().stream().allMatch(actualParams ->
-                    actualParams.equals(expectedParams)));
-        }
     }
 
     @Test
-    @Parameters(method = "testData")
-    public void testStoreOriginalPayloadOnS3Failure(PayloadStore payloadStore, ServerSideEncryptionStrategy awsKmsKeyId, S3Dao mockS3Dao) {
+    public void testStoreOriginalPayloadOnS3Failure() {
         doThrow(SdkException.create("S3 Exception", new Throwable()))
-                .when(mockS3Dao)
+                .when(s3Dao)
                 .storeTextInS3(
                         any(String.class),
                         any(String.class),
-                        // Can be String or null
-                        any(),
                         any(String.class));
 
         exception.expect(SdkException.class);
diff --git a/src/test/java/software/amazon/payloadoffloading/S3DaoTest.java b/src/test/java/software/amazon/payloadoffloading/S3DaoTest.java
