Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Files not being created with the proper ACL #927

Closed
devinnasar opened this issue Sep 4, 2020 · 12 comments
Closed

Files not being created with the proper ACL #927

devinnasar opened this issue Sep 4, 2020 · 12 comments

Comments

@devinnasar
Copy link

Hello,

I am using django-storages with s3. I've noticed that files are not being created in my s3 bucket through storages with the proper ACL, though I'm certain that this was working just a little while ago. Can you help me understand what I'm doing wrong? Thanks!

# project structure
.
├── Pipfile
├── Pipfile.lock
├── requirements.txt
└── project
    ├── entrypoint.sh
    ├── manage.py
    ├── media
    ├── static
    ├── project_app
    │   ├── asgi.py
    │   ├── __init__.py
    │   ├── __pycache__
    │   ├── urls.py
    │   └── wsgi.py
    └── app
        ├── admin.py
        ├── apps.py
        ├── forms.py
        ├── __init__.py
        ├── migrations
        ├── models.py
        ├── __pycache__
        ├── serializers.py
        ├── storage_backends.py
        ├── tests.py
        ├── urls.py
        └── views.py

##### project_app/settings.py
# aws settings
# AWS credentials collected from profile by setting AWS_PROFILE='my-profile' as env variable
AWS_STORAGE_BUCKET_NAME = os.getenv('AWS_STORAGE_BUCKET_NAME')
AWS_DEFAULT_ACL = None
AWS_S3_CUSTOM_DOMAIN = f'{AWS_STORAGE_BUCKET_NAME}.s3.amazonaws.com'
AWS_S3_OBJECT_PARAMETERS = {'CacheControl': 'max-age=86400'}

# s3 static settings
STATIC_LOCATION = 'static'
STATIC_URL = f'https://{AWS_S3_CUSTOM_DOMAIN}/{STATIC_LOCATION}/'
STATICFILES_STORAGE = 'app.storage_backends.StaticStorage'

# s3 public media settings
PUBLIC_MEDIA_LOCATION = 'media'
MEDIA_URL = f'https://{AWS_S3_CUSTOM_DOMAIN}/{PUBLIC_MEDIA_LOCATION}/'
DEFAULT_FILE_STORAGE = 'app.storage_backends.PublicMediaStorage'


##### app/storage_backends.py
from storages.backends.s3boto3 import S3Boto3Storage
from django.conf import settings

class StaticStorage(S3Boto3Storage):
    location = 'static'
    default_acl = 'public-read'


class PublicMediaStorage(S3Boto3Storage):
    location = 'media'
    default_acl = 'public-read'
    file_overwrite = False

My s3 bucket settings:

  • The profile has full access to the bucket

AWS console s3 settings images

@owurman
Copy link

owurman commented Sep 4, 2020

Version 1.10 broke S3 for me entirely, possibly because of an ACL issue (I haven't dug into it, but it would check out--I'm getting PutObject denied which would happen if there's an ACL other than None given my bucket policies). Rolling back to 1.9.1 fixes it.

@andrekeller
Copy link

Maybe related to AWS_DEFAULT_ACL being replaced,

i.e.

AWS_S3_OBJECT_PARAMETERS = {
    'ACL': 'public-read',
}

instead of

AWS_DEFAULT_ACL = 'public-read'

You might want to have a look at https://github.com/jschneier/django-storages/blob/master/CHANGELOG.rst quite a few breaking changes in 1.10.

@rohfle
Copy link

rohfle commented Sep 6, 2020

I have run into this too using a fix from #382

custom_storages.py

class MediaStorage(S3Boto3Storage):
    location = settings.MEDIAFILES_LOCATION
    default_acl = 'public-read'
    file_overwrite = False
[...]

In e37912b, support for the class attribute default_acl has been removed and replaced with the following (as demonstrated in unit tests)

object_parameters = {
    'ACL': 'public-read',
}

Adding the following to settings.py fixed the issue for me

AWS_S3_OBJECT_PARAMETERS = {
    'ACL': 'public-read',
}

I'm not sure how internal the default_acl flag was, but as it was used by the workaround for #382, it could be useful to add a explicit changelog note about it

Edit: on closer inspection, class attributes that can override settings.py are located in S3Boto3Storage.get_default_settings. There doesn't seem to be any explicit documentation on them.

@WhyNotHugo
Copy link
Contributor

How do you pass that when creating an instance?

Did 1.10 introduce anything aside from dropping a bunch of very useful features?

ArchiveStorage = S3Boto3Storage(
    bucket_name=f"archives.{settings.SITE_SLUG}",
    default_acl="private",
    querystring_auth=True,
    querystring_expire=3600 * 48,
)

@rohfle
Copy link

rohfle commented Sep 7, 2020

On closer inspection, I don't think that features were dropped... the API was changed that's all.

See S3Boto3Storage.get_default_settings and BaseStorage for how to override settings.py-based configuration when creating an instance.

Try setting object_parameters. to {'ACL': 'private'}

@WhyNotHugo
Copy link
Contributor

@rohfle Thanks!

@Ouradze Ouradze mentioned this issue Sep 8, 2020
@Ouradze
Copy link

Ouradze commented Sep 8, 2020

Hi, maybe the documentation could be updated to explain how to properly override the S3Boto3Storage class ?

@jschneier
Copy link
Owner

jschneier commented Sep 9, 2020

@Ouradze sure. Is there anything in particular you are missing. The simplest example of porting ACL is this

AWS_S3_OBJECT_PARAMETERS = {
  'ACL': ...your acl setting here
}

Overriding will allow you to do things like use different parameter values based on filename.

@Ouradze
Copy link

Ouradze commented Sep 9, 2020

On closer inspection, I don't think that features were dropped... the API was changed that's all.

See S3Boto3Storage.get_default_settings and BaseStorage for how to override settings.py-based configuration when creating an instance.

Try setting object_parameters. to {'ACL': 'private'}

Thanks @jschneier ! I used this for now as we have multiple classes with different ACLs.

@devinnasar
Copy link
Author

Hello,

Thank you all for your feedback. I read the changelog regarding default_acl and read that the intended way to work with it on a static/media basis was to subclass S3Boto3Storage. I've done that in the following manner:

from storages.backends.s3boto3 import S3Boto3Storage
from django.conf import settings

class StaticStorage(S3Boto3Storage):
    location = settings.STATIC_LOCATION

    def get_object_parameters(self, name):

        s3_object_params = {
            "ACL": "public-read"
        }

        return {**s3_object_params, **self.object_parameters.copy()}

class PublicMediaStorage(S3Boto3Storage):
    location = settings.PUBLIC_MEDIA_LOCATION
    file_overwrite = False

    def get_object_parameters(self, name):

        s3_object_params = {
            "ACL": "public-read"
        }

        return {**s3_object_params, **self.object_parameters.copy()}

I noticed that self.object_parameters is used in a number of locations, so I figured that I would set the ACL in each storage class separately and then merge the params dict with self.object_parameters so it can pass that along to whatever needs to read object params. Also this lets me change ACL on these separately in case I would ever want to. Does this seem like a valid approach? It's definitely working for now, but I wanted to get a check before I stick with this method.

@jschneier
Copy link
Owner

I have release 1.10.1 that restores handling of AWS_DEFAULT_ACL / default_acl. Sorry for the pain.

@asencis
Copy link

asencis commented Sep 16, 2020

@jschneier Thanks Josh, we both simultaneously hate and love you. Appreciate all the hard work on django-storages! Love not hate. 😀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants