diff --git a/synapseclient/core/remote_file_storage_wrappers.py b/synapseclient/core/remote_file_storage_wrappers.py index 8d52b35d5..ac67d9f02 100644 --- 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 index 4872c3604..03a1916d6 100644 --- 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 index 5fefeb633..4486e1910 100644 --- 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...?