Remove "Storage Admin" role requirement.

[wooyek]

Call to get_bucket results 403 Caller does not have storage.buckets.get
access to bucket. If permissions are limited.

If auto_create_bucket bucket is not True, we don't have to check if
bucket exits just in case we would want to create it - which we don't,
cause we don't have privileges anyway

[codecov-io]

Codecov Report

Merging #412 into master will decrease coverage by 0.55%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #412      +/-   ##
==========================================
- Coverage    77.1%   76.54%   -0.56%     
==========================================
  Files          10       11       +1     
  Lines        1520     1599      +79     
==========================================
+ Hits         1172     1224      +52     
- Misses        348      375      +27

Impacted Files	Coverage Δ
storages/backends/gcloud.py	95.05% <100%> (+0.11%)	⬆️
storages/backends/s3boto.py	88.03% <0%> (-0.67%)	⬇️
storages/backends/s3boto3.py	87.12% <0%> (-0.6%)	⬇️
storages/utils.py	96.87% <0%> (-0.1%)	⬇️
storages/backends/gs.py	69.73% <0%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 82f31b8...8c10a2d. Read the comment docs.

[wooyek]

Phew! I had to clean up some other things to pass the tests:

• cryptography dropped support for py33 with version 2 so there is a constraint in the tox
• a minor linting problem with ftp backend, that I did not touch originally

Please tell me If there is something more I should do to make this PR accepted

[wooyek]

@jschneier Please tell me If there is something more I should do to make this PR accepted

[sww314]

@wooyek Can you reimplement this against master? I tried to review it, but there is over a year of changes and I am not sure what changed. There are at least a few unrelated changes it would be great to separate out.

[wooyek]

I'm confused I thought I am implementing this against master. Please see the changes here:

https://github.com/jschneier/django-storages/pull/412/files

I am happy to try again if that's not sufficient.

[jschneier]

Instead of adding a setting maybe we just want to catch the error, log and return the proxy?

[wooyek]

IMHO setting is more explicit and by default does not change current behavior.

If you want I can rewrite this later on. But I really would like to do this in another PR and release this one as soon as possible. I don't like for production systems to rely on git repos instead of released packages.

[sww314]

Personally, I would be in favor of removal of the GS_AUTO_CREATE_BUCKET capability. For the vast majority of use cases, it requires more permissions than needed + is inefficient because it creates a network trip to Google every time to check the bucket status.

Is there a real use case for this capability? If you are dynamically creating buckets - you probably need more logic than this provides anyway.

[jschneier]

Yeah. Was thinking that as well.

…

On Aug 14, 2018, at 11:14 AM, Scott White ***@***.***> wrote: Personally, I would be in favor of removal of the GS_AUTO_CREATE_BUCKET capability. For the vast majority of use cases, it requires more permissions than needed + is inefficient because it creates a network trip to Google every time to check the bucket status. Is there a real use case for this capability? If you are dynamically creating buckets - you probably need more logic than this provides anyway. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#412 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACJB2CpJQw0PQj81iOWq84mVnBBoQWM-ks5uQulFgaJpZM4QHWVf>.

[wooyek]

+1 to the removal of GS_AUTO_CREATE_BUCKET, but please let's do that in another PR. Let's not make this PR a all-in-one constantly postponed PR.

Let's focus on the issue here, working around 403 that is not caused by lack od create permissions but from lack of Caller does not have storage.buckets.get access to bucket permission. Let's solve that and move on to the next improvements.

[jschneier]

If you want to catch Forbidden and return the bucket then I will merge that. I don't want to add another setting if we are going to go ahead and then remove the auto_create_bucket that would then require deprecating the setting. That will fix this issue.

[jschneier]

I see. The behavior in S3Boto3 is different from in Google Cloud and the S3Boto backend. The first does not hit the API at all unless you can possibly create the bucket. Okay maybe we should make that change instead.

[jschneier]

Fixed by #718.