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

S3 Storage: Support AWS Signature Version 4 query parameters #4868

Closed
darthhexx opened this issue Oct 5, 2023 · 3 comments
Closed

S3 Storage: Support AWS Signature Version 4 query parameters #4868

darthhexx opened this issue Oct 5, 2023 · 3 comments
Assignees
Labels
community:reviewed Issue has been reviewed by the Label Studio Community Team.

Comments

@darthhexx
Copy link
Contributor

Is your feature request related to a problem? Please describe.

When setting STORAGE_TYPE=s3 the requests to /storage-data/uploaded/?filepath=upload/1/somefile result in generated URLs for AWS presigned with the old format: URL?AWSAccessKeyId=AKIA...redacted...&Signature=F1H25pgUkZwDhV0S2b%2F48rPibWk%3D&Expires=1696592186.

This is due to this call to the Django S3Boto3Storage class and, because the setting for the signature version in the Django setting AWS_S3_SIGNATURE_VERSION is None, the old signature version is being used.

Describe the solution you'd like

We should introduce an environment variable that enable us to override the setting to s3v4, the only supported version in AWS S3, but keeps the existing behaviour by default so as not to break existing setups.

Describe alternatives you've considered

I think this is the best option, as it keeps the existing behaviour but allows it to be overridden to the latest version.

@hogepodge hogepodge added the community:reviewed Issue has been reviewed by the Label Studio Community Team. label Oct 12, 2023
@hogepodge
Copy link
Contributor

Thanks for catching this. I'll bring it to the engineering team to get the work scoped out.

@darthhexx
Copy link
Contributor Author

Thanks @hogepodge. I have a proposed fix in #4869 that I am using in my deployment, which works as expected.

@bmartel
Copy link
Contributor

bmartel commented Oct 24, 2023

Thanks a bunch @darthhexx. This has been merged back in, closing this issue!

@bmartel bmartel closed this as completed Oct 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community:reviewed Issue has been reviewed by the Label Studio Community Team.
Projects
None yet
Development

No branches or pull requests

4 participants