apply bucket-owner-full-control on boto synapse upload

Sage-Bionetworks · May 18, 2021 · b37e145 · b37e145
1 parent 87530e3
commit b37e145
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 2 deletions.
diff --git a/synapseclient/core/remote_file_storage_wrappers.py b/synapseclient/core/remote_file_storage_wrappers.py
@@ -132,7 +132,8 @@ def upload_file(bucket, endpoint_url, remote_file_key, upload_file_path,
             upload_file_path,
             remote_file_key,
             Callback=progress_callback,
-            Config=transfer_config
+            Config=transfer_config,
+            ExtraArgs={'ACL': 'bucket-owner-full-control'},
         )
         return upload_file_path
 

diff --git a/tests/integration/synapseclient/core/test_external_storage.py b/tests/integration/synapseclient/core/test_external_storage.py
@@ -8,6 +8,7 @@
 
 import pytest
 import unittest
+from unittest import mock
 
 try:
     boto3 = importlib.import_module('boto3')
@@ -181,3 +182,37 @@ def test_sts_external_storage_location(self):
         retrieved_file_entity = self.syn.get(file_entity['id'])
         with open(retrieved_file_entity.path, 'r') as f:
             assert file_contents == f.read()
+
+    def test_boto_upload__acl(self):
+        """Verify when we store a Synapse object using boto we apply a bucket-owner-full-control ACL to the object"""
+        bucket_name, _ = get_aws_env()
+        _, folder, storage_location_id = self._configure_storage_location(sts_enabled=True)
+
+        file_contents = str(uuid.uuid4())
+        upload_file = self._make_temp_file(contents=file_contents)
+
+        # mock the sts setting so that we upload this file using boto regardless of test configuration
+        with mock.patch.object(self.syn, 'use_boto_sts_transfers', new_callable=mock.PropertyMock(return_value=True)):
+            file = self.syn.store(File(path=upload_file.name, parent=folder))
+
+        s3_read_client = boto3.client('s3', **get_aws_env()[1])
+        bucket_acl = s3_read_client.get_bucket_acl(Bucket=bucket_name)
+        bucket_grantee = bucket_acl['Grants'][0]['Grantee']
+        assert bucket_grantee['Type'] == 'CanonicalUser'
+        bucket_owner_id = bucket_grantee['ID']
+
+        # with_retry to avoid acidity issues of an S3 put
+        object_acl = with_retry(
+            lambda: s3_read_client.get_object_acl(
+                Bucket=bucket_name,
+                Key=file['_file_handle']['key']
+            ),
+            retry_exceptions=[s3_read_client.exceptions.NoSuchKey]
+        )
+        grants = object_acl['Grants']
+        assert len(grants) == 1
+        grant = grants[0]
+        grantee = grant['Grantee']
+        assert grantee['Type'] == 'CanonicalUser'
+        assert grantee['ID'] == bucket_owner_id
+        assert grant['Permission'] == 'FULL_CONTROL'
diff --git a/tests/unit/synapseclient/core/unit_test_remote_storage_file_wrappers.py b/tests/unit/synapseclient/core/unit_test_remote_storage_file_wrappers.py
@@ -197,7 +197,8 @@ def _upload_test(**kwargs):
                 upload_file_path,
                 remote_file_key,
                 Callback=progress_callback,
-                Config=transfer_config
+                Config=transfer_config,
+                ExtraArgs={'ACL': 'bucket-owner-full-control'},
             )
 
             # why do we return something we passed...?