Warn about upcoming change to AWS_DEFAULT_ACL; allow None in AWS_DEFAULT_ACL

storages/backends/s3boto3.py

@@ -2,9 +2,11 @@
 import os
 import posixpath
 import threading
+import warnings
 from gzip import GzipFile
 from tempfile import SpooledTemporaryFile
 
+from django.conf import settings as django_settings
 from django.core.exceptions import ImproperlyConfigured, SuspiciousOperation
 from django.core.files.base import File
 from django.core.files.storage import Storage
@@ -115,7 +117,8 @@ def write(self, content):
         self._is_dirty = True
         if self._multipart is None:
             parameters = self._storage.object_parameters.copy()
-            parameters['ACL'] = self._storage.default_acl
+            if self._storage.default_acl:
+                parameters['ACL'] = self._storage.default_acl
             parameters['ContentType'] = (mimetypes.guess_type(self.obj.key)[0] or
                                          self._storage.default_content_type)
             if self._storage.reduced_redundancy:
@@ -258,6 +261,17 @@ def __init__(self, acl=None, bucket=None, **settings):
             self.config = Config(s3={'addressing_style': self.addressing_style},
                                  signature_version=self.signature_version)
 
+        # warn about upcoming change in default AWS_DEFAULT_ACL setting
+        if not hasattr(django_settings, 'AWS_DEFAULT_ACL'):
+            warnings.warn(
+                "The default behavior of S3Boto3Storage is insecure and will change "
+                "in django-storages 2.0. By default files and new buckets are saved "
+                "with an ACL of 'public-read' (globally publicly readable). Version 2.0 will "
+                "default to using the bucket's ACL. To opt into the new behavior set "
+                "AWS_DEFAULT_ACL = None, otherwise to silence this warning explicitly "
+                "set AWS_DEFAULT_ACL."
+            )
+
     @property
     def connection(self):
         # TODO: Support host, port like in s3boto
@@ -350,7 +364,10 @@ def _get_or_create_bucket(self, name):
                     #
                     # Also note that Amazon specifically disallows "us-east-1" when passing bucket
                     # region names; LocationConstraint *must* be blank to create in US Standard.
-                    bucket_params = {'ACL': self.bucket_acl}
+                    if self.bucket_acl:
+                        bucket_params = {'ACL': self.bucket_acl}
+                    else:
+                        bucket_params = {}
                     region_name = self.connection.meta.client.meta.region_name
                     if region_name != 'us-east-1':
                         bucket_params['CreateBucketConfiguration'] = {

tests/test_s3boto3.py

@@ -90,6 +90,25 @@ def test_storage_save(self):
             }
         )
 
+    def test_storage_save_with_acl(self):
+        """
+        Test saving a file with user defined ACL.
+        """
+        name = 'test_storage_save.txt'
+        content = ContentFile('new content')
+        self.storage.default_acl = 'private'
+        self.storage.save(name, content)
+        self.storage.bucket.Object.assert_called_once_with(name)
+
+        obj = self.storage.bucket.Object.return_value
+        obj.upload_fileobj.assert_called_with(
+            content.file,
+            ExtraArgs={
+                'ContentType': 'text/plain',
+                'ACL': 'private',
+            }
+        )
+
     def test_content_type(self):
         """
         Test saving a file with a None content type.
@@ -238,6 +257,24 @@ def test_auto_creating_bucket(self):
             }
         )
 
+    def test_auto_creating_bucket_with_acl(self):
+        self.storage.auto_create_bucket = True
+        self.storage.bucket_acl = 'public-read'
+        Bucket = mock.MagicMock()
+        self.storage._connections.connection.Bucket.return_value = Bucket
+        self.storage._connections.connection.meta.client.meta.region_name = 'sa-east-1'
+
+        Bucket.meta.client.head_bucket.side_effect = ClientError({'Error': {},
+                                                                  'ResponseMetadata': {'HTTPStatusCode': 404}},
+                                                                 'head_bucket')
+        self.storage._get_or_create_bucket('testbucketname')
+        Bucket.create.assert_called_once_with(
+            ACL='public-read',
+            CreateBucketConfiguration={
+                'LocationConstraint': 'sa-east-1',
+            }
+        )
+
     def test_storage_exists(self):
         self.assertTrue(self.storage.exists("file.txt"))
         self.storage.connection.meta.client.head_object.assert_called_with(